Re: Review Request 63271: Windows: Added `os::set_job_mem_limit` to stout.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63271/#review190955 --- Ship it! Ship It! - Aaron Wood On Nov. 14, 2017, 12:20 a.m., Andrew Schwartzmeyer wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/63271/ > --- > > (Updated Nov. 14, 2017, 12:20 a.m.) > > > Review request for mesos, Aaron Wood, Akash Gupta, Jeff Coffler, Jie Yu, John > Kordich, Joseph Wu, and Michael Park. > > > Repository: mesos > > > Description > --- > > This is used to set a hard cap on the memory usage for a job object. > > > Diffs > - > > 3rdparty/stout/include/stout/windows/os.hpp > 09ddec6d69472cd13b453fe1a77fdbe343fc23c8 > > > Diff: https://reviews.apache.org/r/63271/diff/2/ > > > Testing > --- > > > Thanks, > > Andrew Schwartzmeyer > >
Re: Review Request 63279: Increased check tests task resources for Windows.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63279/#review189972 --- Ship it! Ship It! - Aaron Wood On Nov. 2, 2017, 8:40 p.m., Andrew Schwartzmeyer wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/63279/ > --- > > (Updated Nov. 2, 2017, 8:40 p.m.) > > > Review request for mesos, Aaron Wood, Akash Gupta, Alexander Rukletsov, Jeff > Coffler, Gaston Kleiman, Jie Yu, John Kordich, Joseph Wu, Li Li, and Michael > Park. > > > Bugs: MESOS-6690 > https://issues.apache.org/jira/browse/MESOS-6690 > > > Repository: mesos > > > Description > --- > > Unfortunately, due to PowerShell's resource usage as a .NET program, on > Windows 32 MB is not enough memory to run these tests. One instance of > PowerShell takes > 128 MB, and two instances take > 256 MB. > Realistically, the safe minimum is 512 MB of memory. > > > Diffs > - > > src/tests/check_tests.cpp d3ffc0b0204d021f1ed734ece189d0e3a3fd8844 > > > Diff: https://reviews.apache.org/r/63279/diff/1/ > > > Testing > --- > > Built and ran `mesos-tests.exe` on Windows repeatedly, verified all tests > still pass consistently (likewise for `stout` and `libprocess` tests. > > ~~Currently verifying no breaks on Linux.~~ Built and ran tests on Linux. > > NOTE: There are more check tests that are currently disabled for Windows, > that I think should be enabled, but did immediately work for me, so I've left > them disabled to unblock myself. Similarly, I would ideally like to port the > balloon example framework, and use that to prove the effectiveness of the job > object memory hard-cap. Having not yet ported it though, I manually verified > the effectiveness of the new isolators by launching test CPU and memory test > tasks on a deployed cluster my these changes (and the fact that PowerShell > OOM'ed was a nice verification too). > > > Thanks, > > Andrew Schwartzmeyer > >
Re: Review Request 63278: Windows: Documented the `cpuset` and `mem` isolators.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63278/#review189971 --- Ship it! Ship It! - Aaron Wood On Nov. 2, 2017, 8:40 p.m., Andrew Schwartzmeyer wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/63278/ > --- > > (Updated Nov. 2, 2017, 8:40 p.m.) > > > Review request for mesos, Aaron Wood, Akash Gupta, Jeff Coffler, Jie Yu, John > Kordich, Joseph Wu, Li Li, and Michael Park. > > > Bugs: MESOS-6690 > https://issues.apache.org/jira/browse/MESOS-6690 > > > Repository: mesos > > > Description > --- > > This adds documentation on the usage of the job object isolators, which > enable task limits, as well as the statistics they report. > > > Diffs > - > > docs/isolators/windows.md PRE-CREATION > docs/mesos-containerizer.md 9cb071273a5ce42784457ea01b363911fdb7773d > > > Diff: https://reviews.apache.org/r/63278/diff/1/ > > > Testing > --- > > > Thanks, > > Andrew Schwartzmeyer > >
Re: Review Request 63273: Windows: Added `os::get_job_processes` to stout.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63273/#review189966 --- 3rdparty/stout/include/stout/windows/os.hpp Lines 793 (patched) <https://reviews.apache.org/r/63273/#comment267213> Do you want to start at 4 here instead? It looks like PID 0 on Windows is the `Idle` process and PID 4 is the `System` process. You could cut down on iteration too by incrementing in multiples of 4. It seems that both thread IDs and process IDs are multiples of 4 https://blogs.msdn.microsoft.com/oldnewthing/20080228-00/?p=23283/ They do mention that you should not rely on this behavior which is too bad. If you could always assume this you could reduce your cycles here. - Aaron Wood On Nov. 2, 2017, 8:40 p.m., Andrew Schwartzmeyer wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/63273/ > --- > > (Updated Nov. 2, 2017, 8:40 p.m.) > > > Review request for mesos, Aaron Wood, Akash Gupta, Jeff Coffler, Jie Yu, John > Kordich, Joseph Wu, and Michael Park. > > > Repository: mesos > > > Description > --- > > Returns a `set` for all the processes in a job object. > > > Diffs > - > > 3rdparty/stout/include/stout/windows/os.hpp > 09ddec6d69472cd13b453fe1a77fdbe343fc23c8 > > > Diff: https://reviews.apache.org/r/63273/diff/1/ > > > Testing > --- > > > Thanks, > > Andrew Schwartzmeyer > >
Re: Review Request 63268: Windows: Fixed variable casing in `windows/os.hpp`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63268/#review189963 --- Ship it! Ship It! - Aaron Wood On Nov. 2, 2017, 8:39 p.m., Andrew Schwartzmeyer wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/63268/ > --- > > (Updated Nov. 2, 2017, 8:39 p.m.) > > > Review request for mesos, Aaron Wood, Akash Gupta, Jeff Coffler, Jie Yu, John > Kordich, Joseph Wu, and Michael Park. > > > Repository: mesos > > > Description > --- > > This changes the CamelCase to snake_case per the style guide for stout. > It also adds `::` to uses of `::GetLastError` where it was missing, and > includes `` because it was used but missing. > > > Diffs > - > > 3rdparty/stout/include/stout/windows/os.hpp > 09ddec6d69472cd13b453fe1a77fdbe343fc23c8 > > > Diff: https://reviews.apache.org/r/63268/diff/1/ > > > Testing > --- > > > Thanks, > > Andrew Schwartzmeyer > >
Re: Review Request 63268: Windows: Fixed variable casing in `windows/os.hpp`.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63268/#review189962
---
3rdparty/stout/include/stout/windows/os.hpp
Line 669 (original), 670 (patched)
<https://reviews.apache.org/r/63268/#comment267203>
How about doing `if (job_handle.get_handle()) {` instead?
3rdparty/stout/include/stout/windows/os.hpp
Line 715 (original), 716 (patched)
<https://reviews.apache.org/r/63268/#comment267204>
Might be a bit cleaner to do `if (!result) {`
- Aaron Wood
On Nov. 2, 2017, 8:39 p.m., Andrew Schwartzmeyer wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63268/
> ---
>
> (Updated Nov. 2, 2017, 8:39 p.m.)
>
>
> Review request for mesos, Aaron Wood, Akash Gupta, Jeff Coffler, Jie Yu, John
> Kordich, Joseph Wu, and Michael Park.
>
>
> Repository: mesos
>
>
> Description
> ---
>
> This changes the CamelCase to snake_case per the style guide for stout.
> It also adds `::` to uses of `::GetLastError` where it was missing, and
> includes `` because it was used but missing.
>
>
> Diffs
> -
>
> 3rdparty/stout/include/stout/windows/os.hpp
> 09ddec6d69472cd13b453fe1a77fdbe343fc23c8
>
>
> Diff: https://reviews.apache.org/r/63268/diff/1/
>
>
> Testing
> ---
>
>
> Thanks,
>
> Andrew Schwartzmeyer
>
>
Re: Review Request 60280: Provide full path to the custom executor.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60280/ --- (Updated July 6, 2017, 10:44 p.m.) Review request for mesos, Jie Yu, James Peach, and Zhitao Li. Bugs: MESOS-7703 https://issues.apache.org/jira/browse/MESOS-7703 Repository: mesos Description --- If a framework specifies use of its own executor and sets shell to false the executor is never found. This provides the full path to the executor so that the `execvp` or `execvpe` is successful. Diffs - src/slave/containerizer/mesos/launch.cpp 162ca1c2e Diff: https://reviews.apache.org/r/60280/diff/9/ Testing --- `cd build && cmake .. -DCMAKE_BUILD_TYPE=Release && make -j4` Also spun up a master and agent, connected and sent a task using the UCR (both with and without the use of an OCI image) via our own framework, and checked the sandbox to verify that things went accordingly. Thanks, Aaron Wood
Review Request 60693: Move permissions.hpp under posix
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60693/ --- Review request for mesos, Andrew Schwartzmeyer and Jie Yu. Bugs: MESOS-7764 https://issues.apache.org/jira/browse/MESOS-7764 Repository: mesos Description --- The way permissions are dealt with are not cross-compatible with Windows. Diffs - 3rdparty/stout/include/stout/posix/os.hpp 8511dfd41 src/common/http.cpp 2f7718cbc src/common/protobuf_utils.cpp 4e5ab02c9 src/credentials/credentials.hpp c790793c7 src/tests/containerizer/provisioner_backend_tests.cpp e3516fca0 src/tests/fetcher_tests.cpp 99149baa1 Diff: https://reviews.apache.org/r/60693/diff/1/ Testing --- `mkdir build && cd build && cmake .. && make -j$(nproc) && make check -j$(nproc)` Thanks, Aaron Wood
Re: Review Request 60280: Provide full path to the custom executor.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60280/ --- (Updated July 6, 2017, 3:32 p.m.) Review request for mesos, Jie Yu, James Peach, and Zhitao Li. Bugs: MESOS-7703 https://issues.apache.org/jira/browse/MESOS-7703 Repository: mesos Description --- If a framework specifies use of its own executor and sets shell to false the executor is never found. This provides the full path to the executor so that the `execvp` or `execvpe` is successful. Diffs (updated) - src/slave/containerizer/mesos/launch.cpp 162ca1c2e Diff: https://reviews.apache.org/r/60280/diff/9/ Changes: https://reviews.apache.org/r/60280/diff/8-9/ Testing --- `cd build && cmake .. -DCMAKE_BUILD_TYPE=Release && make -j4` Also spun up a master and agent, connected and sent a task using the UCR (both with and without the use of an OCI image) via our own framework, and checked the sandbox to verify that things went accordingly. Thanks, Aaron Wood
Re: Review Request 60280: Provide full path to the custom executor.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60280/ --- (Updated July 6, 2017, 3:27 p.m.) Review request for mesos, Jie Yu, James Peach, and Zhitao Li. Bugs: MESOS-7703 https://issues.apache.org/jira/browse/MESOS-7703 Repository: mesos Description --- If a framework specifies use of its own executor and sets shell to false the executor is never found. This provides the full path to the executor so that the `execvp` or `execvpe` is successful. Diffs (updated) - src/slave/containerizer/mesos/launch.cpp 162ca1c2e Diff: https://reviews.apache.org/r/60280/diff/8/ Changes: https://reviews.apache.org/r/60280/diff/7-8/ Testing --- `cd build && cmake .. -DCMAKE_BUILD_TYPE=Release && make -j4` Also spun up a master and agent, connected and sent a task using the UCR (both with and without the use of an OCI image) via our own framework, and checked the sandbox to verify that things went accordingly. Thanks, Aaron Wood
Re: Review Request 60280: Provide full path to the custom executor.
> On July 6, 2017, 2:59 a.m., Jie Yu wrote: > > src/slave/containerizer/mesos/launch.cpp > > Lines 808 (patched) > > <https://reviews.apache.org/r/60280/diff/7/?file=1769755#file1769755line808> > > > > This line is longger than 80 chars: > > > > ``` > > Jies-MacBook-Pro:mesos jie$ support/apply-review.sh -r 60280 > > 2017-07-05 19:39:04 URL:https://reviews.apache.org/r/60280/diff/raw/ > > [1397/1397] -> "60280.patch" [1] > > Checking 1 C++ file > > src/slave/containerizer/mesos/launch.cpp:803: Lines should be <= 80 > > characters long [whitespace/line_length] [2] > > src/slave/containerizer/mesos/launch.cpp:808: Lines should be <= 80 > > characters long [whitespace/line_length] [2] > > Total errors found: 1 > > No Python files to lint > > ``` > > > > ALso, we need to test if working_directory is set or not. > > Aaron Wood wrote: > Can't we just rely on `os::which` checking if the working_directory is > set or not? Nevermind, just noticed that `os::which` will grab PATH if `_path` is not set. - Aaron --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60280/#review179745 --- On July 5, 2017, 9:03 p.m., Aaron Wood wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60280/ > --- > > (Updated July 5, 2017, 9:03 p.m.) > > > Review request for mesos, Jie Yu, James Peach, and Zhitao Li. > > > Bugs: MESOS-7703 > https://issues.apache.org/jira/browse/MESOS-7703 > > > Repository: mesos > > > Description > --- > > If a framework specifies use of its own executor and sets shell to false the > executor is never found. This provides the full path to the executor so that > the `execvp` or `execvpe` is successful. > > > Diffs > - > > src/slave/containerizer/mesos/launch.cpp 162ca1c2e > > > Diff: https://reviews.apache.org/r/60280/diff/7/ > > > Testing > --- > > `cd build && cmake .. -DCMAKE_BUILD_TYPE=Release && make -j4` > Also spun up a master and agent, connected and sent a task using the UCR > (both with and without the use of an OCI image) via our own framework, and > checked the sandbox to verify that things went accordingly. > > > Thanks, > > Aaron Wood > >
Re: Review Request 60280: Provide full path to the custom executor.
> On July 6, 2017, 2:59 a.m., Jie Yu wrote: > > src/slave/containerizer/mesos/launch.cpp > > Lines 808 (patched) > > <https://reviews.apache.org/r/60280/diff/7/?file=1769755#file1769755line808> > > > > This line is longger than 80 chars: > > > > ``` > > Jies-MacBook-Pro:mesos jie$ support/apply-review.sh -r 60280 > > 2017-07-05 19:39:04 URL:https://reviews.apache.org/r/60280/diff/raw/ > > [1397/1397] -> "60280.patch" [1] > > Checking 1 C++ file > > src/slave/containerizer/mesos/launch.cpp:803: Lines should be <= 80 > > characters long [whitespace/line_length] [2] > > src/slave/containerizer/mesos/launch.cpp:808: Lines should be <= 80 > > characters long [whitespace/line_length] [2] > > Total errors found: 1 > > No Python files to lint > > ``` > > > > ALso, we need to test if working_directory is set or not. Can't we just rely on `os::which` checking if the working_directory is set or not? - Aaron --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60280/#review179745 --- On July 5, 2017, 9:03 p.m., Aaron Wood wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60280/ > --- > > (Updated July 5, 2017, 9:03 p.m.) > > > Review request for mesos, Jie Yu, James Peach, and Zhitao Li. > > > Bugs: MESOS-7703 > https://issues.apache.org/jira/browse/MESOS-7703 > > > Repository: mesos > > > Description > --- > > If a framework specifies use of its own executor and sets shell to false the > executor is never found. This provides the full path to the executor so that > the `execvp` or `execvpe` is successful. > > > Diffs > - > > src/slave/containerizer/mesos/launch.cpp 162ca1c2e > > > Diff: https://reviews.apache.org/r/60280/diff/7/ > > > Testing > --- > > `cd build && cmake .. -DCMAKE_BUILD_TYPE=Release && make -j4` > Also spun up a master and agent, connected and sent a task using the UCR > (both with and without the use of an OCI image) via our own framework, and > checked the sandbox to verify that things went accordingly. > > > Thanks, > > Aaron Wood > >
Re: Review Request 60280: Provide full path to the custom executor.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60280/ --- (Updated July 5, 2017, 9:03 p.m.) Review request for mesos, Jie Yu, James Peach, and Zhitao Li. Bugs: MESOS-7703 https://issues.apache.org/jira/browse/MESOS-7703 Repository: mesos Description --- If a framework specifies use of its own executor and sets shell to false the executor is never found. This provides the full path to the executor so that the `execvp` or `execvpe` is successful. Diffs (updated) - src/slave/containerizer/mesos/launch.cpp 162ca1c2e Diff: https://reviews.apache.org/r/60280/diff/7/ Changes: https://reviews.apache.org/r/60280/diff/6-7/ Testing --- `cd build && cmake .. -DCMAKE_BUILD_TYPE=Release && make -j4` Also spun up a master and agent, connected and sent a task using the UCR (both with and without the use of an OCI image) via our own framework, and checked the sandbox to verify that things went accordingly. Thanks, Aaron Wood
Re: Review Request 60280: Provide full path to the custom executor.
> On June 30, 2017, 11:48 p.m., Jie Yu wrote: > > src/slave/containerizer/mesos/launch.cpp > > Lines 799 (patched) > > <https://reviews.apache.org/r/60280/diff/3/?file=1758435#file1758435line799> > > > > What if `launchInfo.working_directory()` is not set? Maybe use > > os::realpath here to get the absolute path? > > Aaron Wood wrote: > Sure, I'll alter that. Is it wrong to assume that the working directory > will always be set at this point? I thought that without the sandbox work dir > no container would be able to move forward. > > Jie Yu wrote: > I think there are some cases at the moment where the working_directory is > not set (DEBUG containers): > > https://github.com/apache/mesos/blob/master/src/slave/containerizer/mesos/containerizer.cpp#L1540 > > But I think that's a tech debt itself. Thanks, I didn't realize this existed. - Aaron --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60280/#review179433 ------- On July 3, 2017, 6:35 p.m., Aaron Wood wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60280/ > --- > > (Updated July 3, 2017, 6:35 p.m.) > > > Review request for mesos, Jie Yu, James Peach, and Zhitao Li. > > > Bugs: MESOS-7703 > https://issues.apache.org/jira/browse/MESOS-7703 > > > Repository: mesos > > > Description > --- > > If a framework specifies use of its own executor and sets shell to false the > executor is never found. This provides the full path to the executor so that > the `execvp` or `execvpe` is successful. > > > Diffs > - > > src/slave/containerizer/mesos/launch.cpp 162ca1c2e > > > Diff: https://reviews.apache.org/r/60280/diff/6/ > > > Testing > --- > > `cd build && cmake .. -DCMAKE_BUILD_TYPE=Release && make -j4` > Also spun up a master and agent, connected and sent a task using the UCR > (both with and without the use of an OCI image) via our own framework, and > checked the sandbox to verify that things went accordingly. > > > Thanks, > > Aaron Wood > >
Re: Review Request 60280: Provide full path to the custom executor.
> On July 5, 2017, 4:48 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/launch.cpp
> > Lines 799 (patched)
> > <https://reviews.apache.org/r/60280/diff/6/?file=1768561#file1768561line799>
> >
> > This is still based on the assumption that `launchInfo` has
> > `working_directory()` set. Can you do the following instead:
> > ```
> > Result realpath = os::realpath(executable);
> > if (!realpath.isSome()) {
> > ...
> > }
> > ```
>
> Jie Yu wrote:
> I also realized that using realpath will make the search for PATH not
> possible. This issue is indeed not trivial. The best way I can think of is to
> test if executable is relative and see if it exists in the sandbox or not and
> test if it is executable or not. Alternatively, we can add cwd to PATH, but
> this will polute PATH i would rather avoid.
>
> Do you have other thoughts?
Is it possible to assume that any custom executor would always be in the
sandbox? E.g. assume that a framework will always provide a URI to fetch the
executor and have Mesos execute it in the usual way. If that's the case then
would we need to search `PATH` at all?
- Aaron
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60280/#review179650
---
On July 3, 2017, 6:35 p.m., Aaron Wood wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60280/
> ---
>
> (Updated July 3, 2017, 6:35 p.m.)
>
>
> Review request for mesos, Jie Yu, James Peach, and Zhitao Li.
>
>
> Bugs: MESOS-7703
> https://issues.apache.org/jira/browse/MESOS-7703
>
>
> Repository: mesos
>
>
> Description
> ---
>
> If a framework specifies use of its own executor and sets shell to false the
> executor is never found. This provides the full path to the executor so that
> the `execvp` or `execvpe` is successful.
>
>
> Diffs
> -
>
> src/slave/containerizer/mesos/launch.cpp 162ca1c2e
>
>
> Diff: https://reviews.apache.org/r/60280/diff/6/
>
>
> Testing
> ---
>
> `cd build && cmake .. -DCMAKE_BUILD_TYPE=Release && make -j4`
> Also spun up a master and agent, connected and sent a task using the UCR
> (both with and without the use of an OCI image) via our own framework, and
> checked the sandbox to verify that things went accordingly.
>
>
> Thanks,
>
> Aaron Wood
>
>
Re: Review Request 60546: Harden Mesos when building with cmake.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60546/ --- (Updated July 5, 2017, 4:06 p.m.) Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, and Joseph Wu. Changes --- Use CHECK_CXX_COMPILER_FLAG to detect support for stack protectors and add more documentation surrounding why we want position independent code. Bugs: MESOS-7737 https://issues.apache.org/jira/browse/MESOS-7737 Repository: mesos Description --- Apply stack protectors (stronger stack protectors with newer compilers), position independent code suitable for linking into executables, security warnings, and generate position independent executables. Diffs (updated) - cmake/CompilationConfigure.cmake 3fa2e2f3b cmake/MesosConfigure.cmake 5ecad2c0f Diff: https://reviews.apache.org/r/60546/diff/2/ Changes: https://reviews.apache.org/r/60546/diff/1-2/ Testing --- `mkdir build && cd build && cmake .. && make -j$(nproc) && make check -j$(nproc)` Thanks, Aaron Wood
Re: Review Request 60546: Harden Mesos when building with cmake.
> On July 4, 2017, 3:06 p.m., Benjamin Bannier wrote: > > cmake/CompilationConfigure.cmake > > Lines 213-214 (patched) > > <https://reviews.apache.org/r/60546/diff/1/?file=1767158#file1767158line213> > > > > Not sure this is needed, see below. Relating to my comment below, I saw that `-pie` was not used to generate a fully position independent executable. Only `-fPIC` was used to generate position independent code for dynamic linking. > On July 4, 2017, 3:06 p.m., Benjamin Bannier wrote: > > cmake/MesosConfigure.cmake > > Lines 65 (patched) > > <https://reviews.apache.org/r/60546/diff/1/?file=1767159#file1767159line65> > > > > It appears as if just setting > > > > set(CMAKE_POSITION_INDEPENDENT_CODE TRUE) > > > > here as well would take care of both the compiler and linker flags; > > could you check if it does and potentially streamline this part. It would > > also be great if we could have a comment explaining why we enforce position > > independent code. >From my testing I found that `-fPIC` was always set when >`set(CMAKE_POSITION_INDEPENDENT_CODE TRUE)` is used but `-fPIE` is never set. >I figured I'd add it here since it's very relevant when building static libs. >I'll definitely add more comments surrounding this stuff. Also, I'm assuming that the static libs built here are only for linking into executables. Do you think this will hold true going forward? - Aaron --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60546/#review179581 --- On June 29, 2017, 6:18 p.m., Aaron Wood wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60546/ > --- > > (Updated June 29, 2017, 6:18 p.m.) > > > Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, and Joseph > Wu. > > > Bugs: MESOS-7737 > https://issues.apache.org/jira/browse/MESOS-7737 > > > Repository: mesos > > > Description > --- > > Apply stack protectors (stronger stack protectors with newer compilers), > position independent code suitable for linking into executables, security > warnings, and generate position independent executables. > > > Diffs > - > > cmake/CompilationConfigure.cmake 3fa2e2f3b > cmake/MesosConfigure.cmake 5ecad2c0f > > > Diff: https://reviews.apache.org/r/60546/diff/1/ > > > Testing > --- > > `mkdir build && cd build && cmake .. && make -j$(nproc) && make check > -j$(nproc)` > > > Thanks, > > Aaron Wood > >
Re: Review Request 60280: Provide full path to the custom executor.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60280/ --- (Updated July 3, 2017, 6:35 p.m.) Review request for mesos, Jie Yu, James Peach, and Zhitao Li. Changes --- Exit with failure status if we can't get the absolute path. Bugs: MESOS-7703 https://issues.apache.org/jira/browse/MESOS-7703 Repository: mesos Description --- If a framework specifies use of its own executor and sets shell to false the executor is never found. This provides the full path to the executor so that the `execvp` or `execvpe` is successful. Diffs (updated) - src/slave/containerizer/mesos/launch.cpp 162ca1c2e Diff: https://reviews.apache.org/r/60280/diff/6/ Changes: https://reviews.apache.org/r/60280/diff/5-6/ Testing --- `cd build && cmake .. -DCMAKE_BUILD_TYPE=Release && make -j4` Also spun up a master and agent, connected and sent a task using the UCR (both with and without the use of an OCI image) via our own framework, and checked the sandbox to verify that things went accordingly. Thanks, Aaron Wood
Re: Review Request 60280: Provide full path to the custom executor.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60280/ --- (Updated July 3, 2017, 4:18 p.m.) Review request for mesos, Jie Yu, James Peach, and Zhitao Li. Changes --- Fix comment. Bugs: MESOS-7703 https://issues.apache.org/jira/browse/MESOS-7703 Repository: mesos Description --- If a framework specifies use of its own executor and sets shell to false the executor is never found. This provides the full path to the executor so that the `execvp` or `execvpe` is successful. Diffs (updated) - src/slave/containerizer/mesos/launch.cpp 162ca1c2e Diff: https://reviews.apache.org/r/60280/diff/5/ Changes: https://reviews.apache.org/r/60280/diff/4-5/ Testing --- `cd build && cmake .. -DCMAKE_BUILD_TYPE=Release && make -j4` Also spun up a master and agent, connected and sent a task using the UCR (both with and without the use of an OCI image) via our own framework, and checked the sandbox to verify that things went accordingly. Thanks, Aaron Wood
Re: Review Request 60280: Provide full path to the custom executor.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60280/ --- (Updated July 3, 2017, 4 p.m.) Review request for mesos, Jie Yu, James Peach, and Zhitao Li. Bugs: MESOS-7703 https://issues.apache.org/jira/browse/MESOS-7703 Repository: mesos Description --- If a framework specifies use of its own executor and sets shell to false the executor is never found. This provides the full path to the executor so that the `execvp` or `execvpe` is successful. Diffs (updated) - src/slave/containerizer/mesos/launch.cpp 162ca1c2e Diff: https://reviews.apache.org/r/60280/diff/4/ Changes: https://reviews.apache.org/r/60280/diff/3-4/ Testing --- `cd build && cmake .. -DCMAKE_BUILD_TYPE=Release && make -j4` Also spun up a master and agent, connected and sent a task using the UCR (both with and without the use of an OCI image) via our own framework, and checked the sandbox to verify that things went accordingly. Thanks, Aaron Wood
Re: Review Request 60280: Provide full path to the custom executor.
> On June 30, 2017, 11:48 p.m., Jie Yu wrote: > > src/slave/containerizer/mesos/launch.cpp > > Lines 799 (patched) > > <https://reviews.apache.org/r/60280/diff/3/?file=1758435#file1758435line799> > > > > What if `launchInfo.working_directory()` is not set? Maybe use > > os::realpath here to get the absolute path? Sure, I'll alter that. Is it wrong to assume that the working directory will always be set at this point? I thought that without the sandbox work dir no container would be able to move forward. - Aaron --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60280/#review179433 ------- On June 22, 2017, 3:20 p.m., Aaron Wood wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60280/ > --- > > (Updated June 22, 2017, 3:20 p.m.) > > > Review request for mesos, Jie Yu, James Peach, and Zhitao Li. > > > Bugs: MESOS-7703 > https://issues.apache.org/jira/browse/MESOS-7703 > > > Repository: mesos > > > Description > --- > > If a framework specifies use of its own executor and sets shell to false the > executor is never found. This provides the full path to the executor so that > the `execvp` or `execvpe` is successful. > > > Diffs > - > > src/slave/containerizer/mesos/launch.cpp 162ca1c2e > > > Diff: https://reviews.apache.org/r/60280/diff/3/ > > > Testing > --- > > `cd build && cmake .. -DCMAKE_BUILD_TYPE=Release && make -j4` > Also spun up a master and agent, connected and sent a task using the UCR > (both with and without the use of an OCI image) via our own framework, and > checked the sandbox to verify that things went accordingly. > > > Thanks, > > Aaron Wood > >
Review Request 60546: Harden Mesos when building with cmake.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60546/ --- Review request for mesos, Andrew Schwartzmeyer and Joseph Wu. Bugs: MESOS-7737 https://issues.apache.org/jira/browse/MESOS-7737 Repository: mesos Description --- Apply stack protectors (stronger stack protectors with newer compilers), position independent code suitable for linking into executables, security warnings, and generate position independent executables. Diffs - cmake/CompilationConfigure.cmake 3fa2e2f3b cmake/MesosConfigure.cmake 5ecad2c0f Diff: https://reviews.apache.org/r/60546/diff/1/ Testing --- `mkdir build && cd build && cmake .. && make -j$(nproc) && make check -j$(nproc)` Thanks, Aaron Wood
Re: Review Request 60280: Provide full path to the custom executor.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60280/ --- (Updated June 22, 2017, 3:20 p.m.) Review request for mesos, Jie Yu, James Peach, and Zhitao Li. Bugs: MESOS-7703 https://issues.apache.org/jira/browse/MESOS-7703 Repository: mesos Description (updated) --- If a framework specifies use of its own executor and sets shell to false the executor is never found. This provides the full path to the executor so that the `execvp` or `execvpe` is successful. Diffs - src/slave/containerizer/mesos/launch.cpp 162ca1c2e Diff: https://reviews.apache.org/r/60280/diff/3/ Testing --- `cd build && cmake .. -DCMAKE_BUILD_TYPE=Release && make -j4` Also spun up a master and agent, connected and sent a task using the UCR (both with and without the use of an OCI image) via our own framework, and checked the sandbox to verify that things went accordingly. Thanks, Aaron Wood
Re: Review Request 60280: Provide full path to the custom executor.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60280/ --- (Updated June 22, 2017, 3:19 p.m.) Review request for mesos, Jie Yu, James Peach, and Zhitao Li. Bugs: MESOS-7703 https://issues.apache.org/jira/browse/MESOS-7703 Repository: mesos Description --- If a framework specifies use of its own executor and sets shell to false the executor is never found. Additionally, the name of the binary is never passed as an argument so executors making use of argv[0] will fail. This provides the full path to the executor so that the `execvp` or `execvpe` is successful. The name of the binary is also passed as the first argument for cases where there is no shell used. Diffs (updated) - src/slave/containerizer/mesos/launch.cpp 162ca1c2e Diff: https://reviews.apache.org/r/60280/diff/3/ Changes: https://reviews.apache.org/r/60280/diff/2-3/ Testing --- `cd build && cmake .. -DCMAKE_BUILD_TYPE=Release && make -j4` Also spun up a master and agent, connected and sent a task using the UCR (both with and without the use of an OCI image) via our own framework, and checked the sandbox to verify that things went accordingly. Thanks, Aaron Wood
Re: Review Request 60280: Provide full path to the custom executor.
> On June 21, 2017, 11:07 p.m., Jie Yu wrote: > > src/slave/containerizer/mesos/launch.cpp > > Lines 671-674 (patched) > > <https://reviews.apache.org/r/60280/diff/1/?file=1757586#file1757586line671> > > > > Sorry, I got confused. User should be the one setting argv[0]. So we > > don't need to change the code here. No problem. So every time someone wants to make use of `argv[0]` in their executor somewhere they'll have to construct that argument and send it along in the protos. Why not take care of it here so that it's less to do (and less going over the wire) from the scheduler's side? > On June 21, 2017, 11:07 p.m., Jie Yu wrote: > > src/slave/containerizer/mesos/launch.cpp > > Lines 801 (patched) > > <https://reviews.apache.org/r/60280/diff/1/?file=1757586#file1757586line804> > > > > OK, what if 'executable' has an absolute path? I guess this would break in that case. My thought was that no one would know the absolute path ahead of time. For example, how would a user know the container ID (not even generated yet since the executor/task has not been sent to Mesos) to be used in the full absolute path to their executor binary in the sandbox? - Aaron --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60280/#review178586 --- On June 21, 2017, 10:26 p.m., Aaron Wood wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60280/ > --- > > (Updated June 21, 2017, 10:26 p.m.) > > > Review request for mesos, Jie Yu, James Peach, and Zhitao Li. > > > Bugs: MESOS-7703 > https://issues.apache.org/jira/browse/MESOS-7703 > > > Repository: mesos > > > Description > --- > > If a framework specifies use of its own executor and sets shell to false the > executor is never found. Additionally, the name of the binary is never passed > as an argument so executors making use of argv[0] will fail. This provides > the full path to the executor so that the `execvp` or `execvpe` is > successful. The name of the binary is also passed as the first argument for > cases where there is no shell used. > > > Diffs > - > > src/slave/containerizer/mesos/launch.cpp 162ca1c2e > > > Diff: https://reviews.apache.org/r/60280/diff/2/ > > > Testing > --- > > `cd build && cmake .. -DCMAKE_BUILD_TYPE=Release && make -j4` > Also spun up a master and agent, connected and sent a task using the UCR > (both with and without the use of an OCI image) via our own framework, and > checked the sandbox to verify that things went accordingly. > > > Thanks, > > Aaron Wood > >
Re: Review Request 60280: Provide full path to the custom executor.
> On June 21, 2017, 9:54 p.m., Jie Yu wrote: > > src/slave/containerizer/mesos/launch.cpp > > Lines 801 (patched) > > <https://reviews.apache.org/r/60280/diff/1/?file=1757586#file1757586line804> > > > > Why this is necessary? I think execvp will look into the current > > working directory? > > Aaron Wood wrote: > I found that this was not true. This is the real core of the fix. It > seems that exec will not look in the current directory, only in PATH. `The file is sought in the colon-separated list of directory pathnames specified in the PATH environment variable. If this variable isn't defined, the path list defaults to the current directory followed by the list of directories returned by confstr(_CS_PATH). (This confstr(3) call typically returns the value "/bin:/usr/bin".)` - Aaron --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60280/#review178562 --- On June 21, 2017, 10:26 p.m., Aaron Wood wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60280/ > --- > > (Updated June 21, 2017, 10:26 p.m.) > > > Review request for mesos, Jie Yu, James Peach, and Zhitao Li. > > > Bugs: MESOS-7703 > https://issues.apache.org/jira/browse/MESOS-7703 > > > Repository: mesos > > > Description > --- > > If a framework specifies use of its own executor and sets shell to false the > executor is never found. Additionally, the name of the binary is never passed > as an argument so executors making use of argv[0] will fail. This provides > the full path to the executor so that the `execvp` or `execvpe` is > successful. The name of the binary is also passed as the first argument for > cases where there is no shell used. > > > Diffs > - > > src/slave/containerizer/mesos/launch.cpp 162ca1c2e > > > Diff: https://reviews.apache.org/r/60280/diff/2/ > > > Testing > --- > > `cd build && cmake .. -DCMAKE_BUILD_TYPE=Release && make -j4` > Also spun up a master and agent, connected and sent a task using the UCR > (both with and without the use of an OCI image) via our own framework, and > checked the sandbox to verify that things went accordingly. > > > Thanks, > > Aaron Wood > >
Re: Review Request 60280: Provide full path to the custom executor.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60280/ --- (Updated June 21, 2017, 10:26 p.m.) Review request for mesos, Jie Yu, James Peach, and Zhitao Li. Bugs: MESOS-7703 https://issues.apache.org/jira/browse/MESOS-7703 Repository: mesos Description --- If a framework specifies use of its own executor and sets shell to false the executor is never found. Additionally, the name of the binary is never passed as an argument so executors making use of argv[0] will fail. This provides the full path to the executor so that the `execvp` or `execvpe` is successful. The name of the binary is also passed as the first argument for cases where there is no shell used. Diffs (updated) - src/slave/containerizer/mesos/launch.cpp 162ca1c2e Diff: https://reviews.apache.org/r/60280/diff/2/ Changes: https://reviews.apache.org/r/60280/diff/1-2/ Testing --- `cd build && cmake .. -DCMAKE_BUILD_TYPE=Release && make -j4` Also spun up a master and agent, connected and sent a task using the UCR (both with and without the use of an OCI image) via our own framework, and checked the sandbox to verify that things went accordingly. Thanks, Aaron Wood
Re: Review Request 60280: Provide full path to the custom executor.
> On June 21, 2017, 7:28 p.m., Aaron Wood wrote:
> > Someone correct me if I'm wrong, but I don't think we need this anymore:
> > ```
> > if (launchInfo.has_working_directory()) {
> > Try chdir = os::chdir(launchInfo.working_directory());
> > if (chdir.isError()) {
> > cerr << "Failed to chdir into current working directory "
> ><< "'" << launchInfo.working_directory() << "': "
> ><< chdir.error() << endl;
> > exitWithStatus(EXIT_FAILURE);
> > }
> > }
> > ```
> > Everyone okay with me removing this?
>
> Jie Yu wrote:
> No, we cannot. We do want the process's cwd to be inside the sandbox.
Okay, will keep it as is then.
- Aaron
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60280/#review178534
---
On June 21, 2017, 7:15 p.m., Aaron Wood wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60280/
> ---
>
> (Updated June 21, 2017, 7:15 p.m.)
>
>
> Review request for mesos, Jie Yu, James Peach, and Zhitao Li.
>
>
> Bugs: MESOS-7703
> https://issues.apache.org/jira/browse/MESOS-7703
>
>
> Repository: mesos
>
>
> Description
> ---
>
> If a framework specifies use of its own executor and sets shell to false the
> executor is never found. Additionally, the name of the binary is never passed
> as an argument so executors making use of argv[0] will fail. This provides
> the full path to the executor so that the `execvp` or `execvpe` is
> successful. The name of the binary is also passed as the first argument for
> cases where there is no shell used.
>
>
> Diffs
> -
>
> src/slave/containerizer/mesos/launch.cpp 162ca1c2e
>
>
> Diff: https://reviews.apache.org/r/60280/diff/1/
>
>
> Testing
> ---
>
> `cd build && cmake .. -DCMAKE_BUILD_TYPE=Release && make -j4`
> Also spun up a master and agent, connected and sent a task using the UCR
> (both with and without the use of an OCI image) via our own framework, and
> checked the sandbox to verify that things went accordingly.
>
>
> Thanks,
>
> Aaron Wood
>
>
Re: Review Request 60280: Provide full path to the custom executor.
> On June 21, 2017, 9:54 p.m., Jie Yu wrote: > > src/slave/containerizer/mesos/launch.cpp > > Lines 801 (patched) > > <https://reviews.apache.org/r/60280/diff/1/?file=1757586#file1757586line804> > > > > Why this is necessary? I think execvp will look into the current > > working directory? I found that this was not true. This is the real core of the fix. It seems that exec will not look in the current directory, only in PATH. - Aaron --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60280/#review178562 ------- On June 21, 2017, 7:15 p.m., Aaron Wood wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60280/ > --- > > (Updated June 21, 2017, 7:15 p.m.) > > > Review request for mesos, Jie Yu, James Peach, and Zhitao Li. > > > Bugs: MESOS-7703 > https://issues.apache.org/jira/browse/MESOS-7703 > > > Repository: mesos > > > Description > --- > > If a framework specifies use of its own executor and sets shell to false the > executor is never found. Additionally, the name of the binary is never passed > as an argument so executors making use of argv[0] will fail. This provides > the full path to the executor so that the `execvp` or `execvpe` is > successful. The name of the binary is also passed as the first argument for > cases where there is no shell used. > > > Diffs > - > > src/slave/containerizer/mesos/launch.cpp 162ca1c2e > > > Diff: https://reviews.apache.org/r/60280/diff/1/ > > > Testing > --- > > `cd build && cmake .. -DCMAKE_BUILD_TYPE=Release && make -j4` > Also spun up a master and agent, connected and sent a task using the UCR > (both with and without the use of an OCI image) via our own framework, and > checked the sandbox to verify that things went accordingly. > > > Thanks, > > Aaron Wood > >
Re: Review Request 60280: Provide full path to the custom executor.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60280/#review178534
---
Someone correct me if I'm wrong, but I don't think we need this anymore:
```
if (launchInfo.has_working_directory()) {
Try chdir = os::chdir(launchInfo.working_directory());
if (chdir.isError()) {
cerr << "Failed to chdir into current working directory "
<< "'" << launchInfo.working_directory() << "': "
<< chdir.error() << endl;
exitWithStatus(EXIT_FAILURE);
}
}
```
Everyone okay with me removing this?
- Aaron Wood
On June 21, 2017, 7:15 p.m., Aaron Wood wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60280/
> ---
>
> (Updated June 21, 2017, 7:15 p.m.)
>
>
> Review request for mesos, Jie Yu, James Peach, and Zhitao Li.
>
>
> Bugs: MESOS-7703
> https://issues.apache.org/jira/browse/MESOS-7703
>
>
> Repository: mesos
>
>
> Description
> ---
>
> If a framework specifies use of its own executor and sets shell to false the
> executor is never found. Additionally, the name of the binary is never passed
> as an argument so executors making use of argv[0] will fail. This provides
> the full path to the executor so that the `execvp` or `execvpe` is
> successful. The name of the binary is also passed as the first argument for
> cases where there is no shell used.
>
>
> Diffs
> -
>
> src/slave/containerizer/mesos/launch.cpp 162ca1c2e
>
>
> Diff: https://reviews.apache.org/r/60280/diff/1/
>
>
> Testing
> ---
>
> `cd build && cmake .. -DCMAKE_BUILD_TYPE=Release && make -j4`
> Also spun up a master and agent, connected and sent a task using the UCR
> (both with and without the use of an OCI image) via our own framework, and
> checked the sandbox to verify that things went accordingly.
>
>
> Thanks,
>
> Aaron Wood
>
>
Review Request 60280: Provide full path to the custom executor.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60280/ --- Review request for mesos, Jie Yu, James Peach, and Zhitao Li. Bugs: MESOS-7703 https://issues.apache.org/jira/browse/MESOS-7703 Repository: mesos Description --- If a framework specifies use of its own executor and sets shell to false the executor is never found. Additionally, the name of the binary is never passed as an argument so executors making use of argv[0] will fail. This provides the full path to the executor so that the `execvp` or `execvpe` is successful. The name of the binary is also passed as the first argument for cases where there is no shell used. Diffs - src/slave/containerizer/mesos/launch.cpp 162ca1c2e Diff: https://reviews.apache.org/r/60280/diff/1/ Testing --- `cd build && cmake .. -DCMAKE_BUILD_TYPE=Release && make -j4` Also spun up a master and agent, connected and sent a task using the UCR (both with and without the use of an OCI image) via our own framework, and checked the sandbox to verify that things went accordingly. Thanks, Aaron Wood
Re: Review Request 59641: Attach latest symlink when executor is registered.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59641/
---
(Updated June 5, 2017, 9:36 p.m.)
Review request for mesos, Andrew Schwartzmeyer, James Peach, Vinod Kone, and
Zhitao Li.
Summary (updated)
-
Attach latest symlink when executor is registered.
Bugs: MESOS-7572
https://issues.apache.org/jira/browse/MESOS-7572
Repository: mesos
Description (updated)
---
This will assist framework developers in making features that need to access
the latest sandbox when hitting various operator API endpoints.
Diffs
-
src/slave/slave.cpp 0c7e5f4ef
src/tests/gc_tests.cpp b8fa6a4b9
Diff: https://reviews.apache.org/r/59641/diff/6/
Testing
---
`mkdir build && cd build && cmake .. && make -j2 && make check -j2`
Checked the original behavior:
```
curl -i
localhost:5051/files/browse?path=/tmp/slave/slaves/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-S0/frameworks/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-0001/executors/testapp-924b5c1524d84676a6a71665e2054d31/runs/cbec99c9-da2d-4aaa-bab7-4cee9a4df031
HTTP/1.1 200 OK
Date: Tue, 30 May 2017 17:43:08 GMT
Content-Length: 644
Content-Type: application/json
[{"gid":"root","mode":"-rw-r--r--","mtime":1496161813.0,"nlink":1,"path":"\/tmp\/slave\/slaves\/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-S0\/frameworks\/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-0001\/executors\/testapp-924b5c1524d84676a6a71665e2054d31\/runs\/cbec99c9-da2d-4aaa-bab7-4cee9a4df031\/stderr","size":255,"uid":"root"},{"gid":"root","mode":"-rw-r--r--","mtime":1496161812.0,"nlink":1,"path":"\/tmp\/slave\/slaves\/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-S0\/frameworks\/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-0001\/executors\/testapp-924b5c1524d84676a6a71665e2054d31\/runs\/cbec99c9-da2d-4aaa-bab7-4cee9a4df031\/stdout","size":1142,"uid":"root"}]
```
Checked the new behavior (this would return a 404 before this patch):
```
curl -i
localhost:5051/files/browse?path=/tmp/slave/slaves/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-S0/frameworks/d3566e51-4d13-4eors/testapp-924b5c1524d84676a6a71665e2054d31/runs/latest
HTTP/1.1 200 OK
Date: Tue, 30 May 2017 17:43:13 GMT
Content-Length: 584
Content-Type: application/json
[{"gid":"root","mode":"-rw-r--r--","mtime":1496161813.0,"nlink":1,"path":"\/tmp\/slave\/slaves\/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-S0\/frameworks\/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-0001\/executors\/testapp-924b5c1524d84676a6a71665e2054d31\/runs\/latest\/stderr","size":255,"uid":"root"},{"gid":"root","mode":"-rw-r--r--","mtime":1496161812.0,"nlink":1,"path":"\/tmp\/slave\/slaves\/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-S0\/frameworks\/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-0001\/executors\/testapp-924b5c1524d84676a6a71665e2054d31\/runs\/latest\/stdout","size":1142,"uid":"root"}]
```
Thanks,
Aaron Wood
Re: Review Request 59641: Follow symlinks when resolving paths specified in the various master/agent endpoints.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59641/
---
(Updated June 2, 2017, 8:59 p.m.)
Review request for mesos, Andrew Schwartzmeyer, James Peach, Vinod Kone, and
Zhitao Li.
Bugs: MESOS-7572
https://issues.apache.org/jira/browse/MESOS-7572
Repository: mesos
Description
---
The main benefit of following symlinks in endpoints such as `/files` is that
frameworks will be able to construct a path to the sandbox much easier. This
will assist framework developers in making features that need to provide a path
when hitting various operator API endpoints. Currently, making use of a path
ending in `runs/latest` throws a 404.
One such application could be a scheduler providing the ability for users to
work with their task's sandbox directly without going to the Mesos UI, API
endpoints, or the actual system themselves.
Diffs (updated)
-
src/slave/slave.cpp 0c7e5f4ef
src/tests/gc_tests.cpp b8fa6a4b9
Diff: https://reviews.apache.org/r/59641/diff/6/
Changes: https://reviews.apache.org/r/59641/diff/5-6/
Testing
---
`mkdir build && cd build && cmake .. && make -j2 && make check -j2`
Checked the original behavior:
```
curl -i
localhost:5051/files/browse?path=/tmp/slave/slaves/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-S0/frameworks/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-0001/executors/testapp-924b5c1524d84676a6a71665e2054d31/runs/cbec99c9-da2d-4aaa-bab7-4cee9a4df031
HTTP/1.1 200 OK
Date: Tue, 30 May 2017 17:43:08 GMT
Content-Length: 644
Content-Type: application/json
[{"gid":"root","mode":"-rw-r--r--","mtime":1496161813.0,"nlink":1,"path":"\/tmp\/slave\/slaves\/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-S0\/frameworks\/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-0001\/executors\/testapp-924b5c1524d84676a6a71665e2054d31\/runs\/cbec99c9-da2d-4aaa-bab7-4cee9a4df031\/stderr","size":255,"uid":"root"},{"gid":"root","mode":"-rw-r--r--","mtime":1496161812.0,"nlink":1,"path":"\/tmp\/slave\/slaves\/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-S0\/frameworks\/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-0001\/executors\/testapp-924b5c1524d84676a6a71665e2054d31\/runs\/cbec99c9-da2d-4aaa-bab7-4cee9a4df031\/stdout","size":1142,"uid":"root"}]
```
Checked the new behavior (this would return a 404 before this patch):
```
curl -i
localhost:5051/files/browse?path=/tmp/slave/slaves/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-S0/frameworks/d3566e51-4d13-4eors/testapp-924b5c1524d84676a6a71665e2054d31/runs/latest
HTTP/1.1 200 OK
Date: Tue, 30 May 2017 17:43:13 GMT
Content-Length: 584
Content-Type: application/json
[{"gid":"root","mode":"-rw-r--r--","mtime":1496161813.0,"nlink":1,"path":"\/tmp\/slave\/slaves\/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-S0\/frameworks\/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-0001\/executors\/testapp-924b5c1524d84676a6a71665e2054d31\/runs\/latest\/stderr","size":255,"uid":"root"},{"gid":"root","mode":"-rw-r--r--","mtime":1496161812.0,"nlink":1,"path":"\/tmp\/slave\/slaves\/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-S0\/frameworks\/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-0001\/executors\/testapp-924b5c1524d84676a6a71665e2054d31\/runs\/latest\/stdout","size":1142,"uid":"root"}]
```
Thanks,
Aaron Wood
Re: Review Request 59641: Follow symlinks when resolving paths specified in the various master/agent endpoints.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59641/
---
(Updated June 2, 2017, 2:28 p.m.)
Review request for mesos, Andrew Schwartzmeyer, James Peach, Vinod Kone, and
Zhitao Li.
Changes
---
Added check to make sure the attached latest symlink is cleaned up.
Bugs: MESOS-7572
https://issues.apache.org/jira/browse/MESOS-7572
Repository: mesos
Description
---
The main benefit of following symlinks in endpoints such as `/files` is that
frameworks will be able to construct a path to the sandbox much easier. This
will assist framework developers in making features that need to provide a path
when hitting various operator API endpoints. Currently, making use of a path
ending in `runs/latest` throws a 404.
One such application could be a scheduler providing the ability for users to
work with their task's sandbox directly without going to the Mesos UI, API
endpoints, or the actual system themselves.
Diffs (updated)
-
src/slave/slave.cpp 14de72fa4
src/tests/gc_tests.cpp b8fa6a4b9
Diff: https://reviews.apache.org/r/59641/diff/5/
Changes: https://reviews.apache.org/r/59641/diff/4-5/
Testing
---
`mkdir build && cd build && cmake .. && make -j2 && make check -j2`
Checked the original behavior:
```
curl -i
localhost:5051/files/browse?path=/tmp/slave/slaves/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-S0/frameworks/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-0001/executors/testapp-924b5c1524d84676a6a71665e2054d31/runs/cbec99c9-da2d-4aaa-bab7-4cee9a4df031
HTTP/1.1 200 OK
Date: Tue, 30 May 2017 17:43:08 GMT
Content-Length: 644
Content-Type: application/json
[{"gid":"root","mode":"-rw-r--r--","mtime":1496161813.0,"nlink":1,"path":"\/tmp\/slave\/slaves\/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-S0\/frameworks\/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-0001\/executors\/testapp-924b5c1524d84676a6a71665e2054d31\/runs\/cbec99c9-da2d-4aaa-bab7-4cee9a4df031\/stderr","size":255,"uid":"root"},{"gid":"root","mode":"-rw-r--r--","mtime":1496161812.0,"nlink":1,"path":"\/tmp\/slave\/slaves\/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-S0\/frameworks\/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-0001\/executors\/testapp-924b5c1524d84676a6a71665e2054d31\/runs\/cbec99c9-da2d-4aaa-bab7-4cee9a4df031\/stdout","size":1142,"uid":"root"}]
```
Checked the new behavior (this would return a 404 before this patch):
```
curl -i
localhost:5051/files/browse?path=/tmp/slave/slaves/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-S0/frameworks/d3566e51-4d13-4eors/testapp-924b5c1524d84676a6a71665e2054d31/runs/latest
HTTP/1.1 200 OK
Date: Tue, 30 May 2017 17:43:13 GMT
Content-Length: 584
Content-Type: application/json
[{"gid":"root","mode":"-rw-r--r--","mtime":1496161813.0,"nlink":1,"path":"\/tmp\/slave\/slaves\/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-S0\/frameworks\/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-0001\/executors\/testapp-924b5c1524d84676a6a71665e2054d31\/runs\/latest\/stderr","size":255,"uid":"root"},{"gid":"root","mode":"-rw-r--r--","mtime":1496161812.0,"nlink":1,"path":"\/tmp\/slave\/slaves\/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-S0\/frameworks\/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-0001\/executors\/testapp-924b5c1524d84676a6a71665e2054d31\/runs\/latest\/stdout","size":1142,"uid":"root"}]
```
Thanks,
Aaron Wood
Re: Review Request 59641: Follow symlinks when resolving paths specified in the various master/agent endpoints.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59641/
---
(Updated June 1, 2017, 9:22 p.m.)
Review request for mesos, Andrew Schwartzmeyer, James Peach, Vinod Kone, and
Zhitao Li.
Bugs: MESOS-7572
https://issues.apache.org/jira/browse/MESOS-7572
Repository: mesos
Description
---
The main benefit of following symlinks in endpoints such as `/files` is that
frameworks will be able to construct a path to the sandbox much easier. This
will assist framework developers in making features that need to provide a path
when hitting various operator API endpoints. Currently, making use of a path
ending in `runs/latest` throws a 404.
One such application could be a scheduler providing the ability for users to
work with their task's sandbox directly without going to the Mesos UI, API
endpoints, or the actual system themselves.
Diffs (updated)
-
src/slave/slave.cpp 14de72fa4
src/tests/gc_tests.cpp b8fa6a4b9
Diff: https://reviews.apache.org/r/59641/diff/4/
Changes: https://reviews.apache.org/r/59641/diff/3-4/
Testing
---
`mkdir build && cd build && cmake .. && make -j2 && make check -j2`
Checked the original behavior:
```
curl -i
localhost:5051/files/browse?path=/tmp/slave/slaves/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-S0/frameworks/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-0001/executors/testapp-924b5c1524d84676a6a71665e2054d31/runs/cbec99c9-da2d-4aaa-bab7-4cee9a4df031
HTTP/1.1 200 OK
Date: Tue, 30 May 2017 17:43:08 GMT
Content-Length: 644
Content-Type: application/json
[{"gid":"root","mode":"-rw-r--r--","mtime":1496161813.0,"nlink":1,"path":"\/tmp\/slave\/slaves\/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-S0\/frameworks\/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-0001\/executors\/testapp-924b5c1524d84676a6a71665e2054d31\/runs\/cbec99c9-da2d-4aaa-bab7-4cee9a4df031\/stderr","size":255,"uid":"root"},{"gid":"root","mode":"-rw-r--r--","mtime":1496161812.0,"nlink":1,"path":"\/tmp\/slave\/slaves\/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-S0\/frameworks\/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-0001\/executors\/testapp-924b5c1524d84676a6a71665e2054d31\/runs\/cbec99c9-da2d-4aaa-bab7-4cee9a4df031\/stdout","size":1142,"uid":"root"}]
```
Checked the new behavior (this would return a 404 before this patch):
```
curl -i
localhost:5051/files/browse?path=/tmp/slave/slaves/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-S0/frameworks/d3566e51-4d13-4eors/testapp-924b5c1524d84676a6a71665e2054d31/runs/latest
HTTP/1.1 200 OK
Date: Tue, 30 May 2017 17:43:13 GMT
Content-Length: 584
Content-Type: application/json
[{"gid":"root","mode":"-rw-r--r--","mtime":1496161813.0,"nlink":1,"path":"\/tmp\/slave\/slaves\/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-S0\/frameworks\/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-0001\/executors\/testapp-924b5c1524d84676a6a71665e2054d31\/runs\/latest\/stderr","size":255,"uid":"root"},{"gid":"root","mode":"-rw-r--r--","mtime":1496161812.0,"nlink":1,"path":"\/tmp\/slave\/slaves\/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-S0\/frameworks\/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-0001\/executors\/testapp-924b5c1524d84676a6a71665e2054d31\/runs\/latest\/stdout","size":1142,"uid":"root"}]
```
Thanks,
Aaron Wood
Re: Review Request 59641: Follow symlinks when resolving paths specified in the various master/agent endpoints.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59641/
---
(Updated May 31, 2017, 6:57 p.m.)
Review request for mesos, Andrew Schwartzmeyer, James Peach, Vinod Kone, and
Zhitao Li.
Bugs: MESOS-7572
https://issues.apache.org/jira/browse/MESOS-7572
Repository: mesos
Description
---
The main benefit of following symlinks in endpoints such as `/files` is that
frameworks will be able to construct a path to the sandbox much easier. This
will assist framework developers in making features that need to provide a path
when hitting various operator API endpoints. Currently, making use of a path
ending in `runs/latest` throws a 404.
One such application could be a scheduler providing the ability for users to
work with their task's sandbox directly without going to the Mesos UI, API
endpoints, or the actual system themselves.
Diffs (updated)
-
src/slave/slave.cpp 14de72fa4
src/tests/files_tests.cpp c703cae03
Diff: https://reviews.apache.org/r/59641/diff/3/
Changes: https://reviews.apache.org/r/59641/diff/2-3/
Testing
---
`mkdir build && cd build && cmake .. && make -j2 && make check -j2`
Checked the original behavior:
```
curl -i
localhost:5051/files/browse?path=/tmp/slave/slaves/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-S0/frameworks/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-0001/executors/testapp-924b5c1524d84676a6a71665e2054d31/runs/cbec99c9-da2d-4aaa-bab7-4cee9a4df031
HTTP/1.1 200 OK
Date: Tue, 30 May 2017 17:43:08 GMT
Content-Length: 644
Content-Type: application/json
[{"gid":"root","mode":"-rw-r--r--","mtime":1496161813.0,"nlink":1,"path":"\/tmp\/slave\/slaves\/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-S0\/frameworks\/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-0001\/executors\/testapp-924b5c1524d84676a6a71665e2054d31\/runs\/cbec99c9-da2d-4aaa-bab7-4cee9a4df031\/stderr","size":255,"uid":"root"},{"gid":"root","mode":"-rw-r--r--","mtime":1496161812.0,"nlink":1,"path":"\/tmp\/slave\/slaves\/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-S0\/frameworks\/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-0001\/executors\/testapp-924b5c1524d84676a6a71665e2054d31\/runs\/cbec99c9-da2d-4aaa-bab7-4cee9a4df031\/stdout","size":1142,"uid":"root"}]
```
Checked the new behavior (this would return a 404 before this patch):
```
curl -i
localhost:5051/files/browse?path=/tmp/slave/slaves/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-S0/frameworks/d3566e51-4d13-4eors/testapp-924b5c1524d84676a6a71665e2054d31/runs/latest
HTTP/1.1 200 OK
Date: Tue, 30 May 2017 17:43:13 GMT
Content-Length: 584
Content-Type: application/json
[{"gid":"root","mode":"-rw-r--r--","mtime":1496161813.0,"nlink":1,"path":"\/tmp\/slave\/slaves\/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-S0\/frameworks\/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-0001\/executors\/testapp-924b5c1524d84676a6a71665e2054d31\/runs\/latest\/stderr","size":255,"uid":"root"},{"gid":"root","mode":"-rw-r--r--","mtime":1496161812.0,"nlink":1,"path":"\/tmp\/slave\/slaves\/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-S0\/frameworks\/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-0001\/executors\/testapp-924b5c1524d84676a6a71665e2054d31\/runs\/latest\/stdout","size":1142,"uid":"root"}]
```
Thanks,
Aaron Wood
Re: Review Request 59641: Follow symlinks when resolving paths specified in the various master/agent endpoints.
> On May 31, 2017, 1:05 a.m., Vinod Kone wrote:
> > src/files/files.cpp
> > Line 874 (original), 874 (patched)
> > <https://reviews.apache.org/r/59641/diff/2/?file=1734876#file1734876line874>
> >
> > I don't follow this. Why are we checking random prefixes for whether
> > they are symlinks on the file system?
I had done this because the path parts that are stored in the map represent the
resolved symlink. The path that's passed into this method here is the symlink
pre-resolved. Without that check the `continue` statement gets hit until we're
out of the loop and then return `None()`.
> On May 31, 2017, 1:05 a.m., Vinod Kone wrote:
> > src/files/files.cpp
> > Lines 892-909 (patched)
> > <https://reviews.apache.org/r/59641/diff/2/?file=1734876#file1734876line892>
> >
> > Instead of doing all this, can we just "attach" "./run/latest" to
> > the files actor when we attach the ".../run/"? See
> > `Framework::addExecutor` and `Framework::recoverExecutor` in slave.cpp.
> > Look for `files->attach(...)`. That to me seems more straightforward?
Sounds good, I'll have a look.
> On May 31, 2017, 1:05 a.m., Vinod Kone wrote:
> > src/files/files.cpp
> > Line 893 (original), 912 (patched)
> > <https://reviews.apache.org/r/59641/diff/2/?file=1734876#file1734876line912>
> >
> > I would do the shadow variable fix in a separate dependent review
> > instead of mixing it here.
I'll revert my naming changes here. Why is it that this kind of change needs a
separate patch?
- Aaron
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59641/#review176415
---
On May 30, 2017, 8:14 p.m., Aaron Wood wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59641/
> ---
>
> (Updated May 30, 2017, 8:14 p.m.)
>
>
> Review request for mesos, Andrew Schwartzmeyer, James Peach, Vinod Kone, and
> Zhitao Li.
>
>
> Bugs: MESOS-7572
> https://issues.apache.org/jira/browse/MESOS-7572
>
>
> Repository: mesos
>
>
> Description
> ---
>
> The main benefit of following symlinks in endpoints such as `/files` is that
> frameworks will be able to construct a path to the sandbox much easier. This
> will assist framework developers in making features that need to provide a
> path when hitting various operator API endpoints. Currently, making use of a
> path ending in `runs/latest` throws a 404.
>
> One such application could be a scheduler providing the ability for users to
> work with their task's sandbox directly without going to the Mesos UI, API
> endpoints, or the actual system themselves.
>
>
> Diffs
> -
>
> src/files/files.cpp b03279ee0
> src/tests/files_tests.cpp c703cae03
>
>
> Diff: https://reviews.apache.org/r/59641/diff/2/
>
>
> Testing
> ---
>
> `mkdir build && cd build && cmake .. && make -j2 && make check -j2`
>
> Checked the original behavior:
> ```
> curl -i
> localhost:5051/files/browse?path=/tmp/slave/slaves/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-S0/frameworks/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-0001/executors/testapp-924b5c1524d84676a6a71665e2054d31/runs/cbec99c9-da2d-4aaa-bab7-4cee9a4df031
> HTTP/1.1 200 OK
> Date: Tue, 30 May 2017 17:43:08 GMT
> Content-Length: 644
> Content-Type: application/json
>
> [{"gid":"root","mode":"-rw-r--r--","mtime":1496161813.0,"nlink":1,"path":"\/tmp\/slave\/slaves\/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-S0\/frameworks\/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-0001\/executors\/testapp-924b5c1524d84676a6a71665e2054d31\/runs\/cbec99c9-da2d-4aaa-bab7-4cee9a4df031\/stderr","size":255,"uid":"root"},{"gid":"root","mode":"-rw-r--r--","mtime":1496161812.0,"nlink":1,"path":"\/tmp\/slave\/slaves\/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-S0\/frameworks\/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-0001\/executors\/testapp-924b5c1524d84676a6a71665e2054d31\/runs\/cbec99c9-da2d-4aaa-bab7-4cee9a4df031\/stdout","size":1142,"uid":"root"}]
> ```
>
> Checked the new behavior (this would return a 404 before this patch):
> ```
> curl -i
> localhost:5051/files/browse?path=/tmp/slave/slaves/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-S0
Re: Review Request 59641: Follow symlinks when resolving paths specified in the various master/agent endpoints.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59641/
---
(Updated May 30, 2017, 8:14 p.m.)
Review request for mesos, Andrew Schwartzmeyer, James Peach, Vinod Kone, and
Zhitao Li.
Bugs: MESOS-7572
https://issues.apache.org/jira/browse/MESOS-7572
Repository: mesos
Description
---
The main benefit of following symlinks in endpoints such as `/files` is that
frameworks will be able to construct a path to the sandbox much easier. This
will assist framework developers in making features that need to provide a path
when hitting various operator API endpoints. Currently, making use of a path
ending in `runs/latest` throws a 404.
One such application could be a scheduler providing the ability for users to
work with their task's sandbox directly without going to the Mesos UI, API
endpoints, or the actual system themselves.
Diffs (updated)
-
src/files/files.cpp b03279ee0
src/tests/files_tests.cpp c703cae03
Diff: https://reviews.apache.org/r/59641/diff/2/
Changes: https://reviews.apache.org/r/59641/diff/1-2/
Testing
---
`mkdir build && cd build && cmake .. && make -j2 && make check -j2`
Checked the original behavior:
```
curl -i
localhost:5051/files/browse?path=/tmp/slave/slaves/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-S0/frameworks/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-0001/executors/testapp-924b5c1524d84676a6a71665e2054d31/runs/cbec99c9-da2d-4aaa-bab7-4cee9a4df031
HTTP/1.1 200 OK
Date: Tue, 30 May 2017 17:43:08 GMT
Content-Length: 644
Content-Type: application/json
[{"gid":"root","mode":"-rw-r--r--","mtime":1496161813.0,"nlink":1,"path":"\/tmp\/slave\/slaves\/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-S0\/frameworks\/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-0001\/executors\/testapp-924b5c1524d84676a6a71665e2054d31\/runs\/cbec99c9-da2d-4aaa-bab7-4cee9a4df031\/stderr","size":255,"uid":"root"},{"gid":"root","mode":"-rw-r--r--","mtime":1496161812.0,"nlink":1,"path":"\/tmp\/slave\/slaves\/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-S0\/frameworks\/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-0001\/executors\/testapp-924b5c1524d84676a6a71665e2054d31\/runs\/cbec99c9-da2d-4aaa-bab7-4cee9a4df031\/stdout","size":1142,"uid":"root"}]
```
Checked the new behavior (this would return a 404 before this patch):
```
curl -i
localhost:5051/files/browse?path=/tmp/slave/slaves/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-S0/frameworks/d3566e51-4d13-4eors/testapp-924b5c1524d84676a6a71665e2054d31/runs/latest
HTTP/1.1 200 OK
Date: Tue, 30 May 2017 17:43:13 GMT
Content-Length: 584
Content-Type: application/json
[{"gid":"root","mode":"-rw-r--r--","mtime":1496161813.0,"nlink":1,"path":"\/tmp\/slave\/slaves\/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-S0\/frameworks\/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-0001\/executors\/testapp-924b5c1524d84676a6a71665e2054d31\/runs\/latest\/stderr","size":255,"uid":"root"},{"gid":"root","mode":"-rw-r--r--","mtime":1496161812.0,"nlink":1,"path":"\/tmp\/slave\/slaves\/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-S0\/frameworks\/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-0001\/executors\/testapp-924b5c1524d84676a6a71665e2054d31\/runs\/latest\/stdout","size":1142,"uid":"root"}]
```
Thanks,
Aaron Wood
Re: Review Request 59641: Follow symlinks when resolving paths specified in the various master/agent endpoints.
> On May 30, 2017, 6:31 p.m., Zhitao Li wrote:
> > Is it possible to add an integration test in `src/tests/files_tests.cpp`?
Sure, I can create a directory and a symlink pointing to it and make sure it
passes. FWIW that test is currently disabled on Windows.
> On May 30, 2017, 6:31 p.m., Zhitao Li wrote:
> > src/files/files.cpp
> > Line 888 (original), 888 (patched)
> > <https://reviews.apache.org/r/59641/diff/1/?file=1734794#file1734794line888>
> >
> > It seems like this `path` variable shadows input argument? I'm not sure
> > whether that's a good idea but I generally found that hard to understand.
> >
> > Consider rename this local variable to something like `resolvedPath`?
I agree, let me change that.
- Aaron
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59641/#review176325
---
On May 30, 2017, 6:10 p.m., Aaron Wood wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59641/
> ---
>
> (Updated May 30, 2017, 6:10 p.m.)
>
>
> Review request for mesos, Andrew Schwartzmeyer, James Peach, Vinod Kone, and
> Zhitao Li.
>
>
> Bugs: MESOS-7572
> https://issues.apache.org/jira/browse/MESOS-7572
>
>
> Repository: mesos
>
>
> Description
> ---
>
> The main benefit of following symlinks in endpoints such as `/files` is that
> frameworks will be able to construct a path to the sandbox much easier. This
> will assist framework developers in making features that need to provide a
> path when hitting various operator API endpoints. Currently, making use of a
> path ending in `runs/latest` throws a 404.
>
> One such application could be a scheduler providing the ability for users to
> work with their task's sandbox directly without going to the Mesos UI, API
> endpoints, or the actual system themselves.
>
>
> Diffs
> -
>
> src/files/files.cpp b03279ee0
>
>
> Diff: https://reviews.apache.org/r/59641/diff/1/
>
>
> Testing
> ---
>
> `mkdir build && cd build && cmake .. && make -j2 && make check -j2`
>
> Checked the original behavior:
> ```
> curl -i
> localhost:5051/files/browse?path=/tmp/slave/slaves/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-S0/frameworks/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-0001/executors/testapp-924b5c1524d84676a6a71665e2054d31/runs/cbec99c9-da2d-4aaa-bab7-4cee9a4df031
> HTTP/1.1 200 OK
> Date: Tue, 30 May 2017 17:43:08 GMT
> Content-Length: 644
> Content-Type: application/json
>
> [{"gid":"root","mode":"-rw-r--r--","mtime":1496161813.0,"nlink":1,"path":"\/tmp\/slave\/slaves\/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-S0\/frameworks\/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-0001\/executors\/testapp-924b5c1524d84676a6a71665e2054d31\/runs\/cbec99c9-da2d-4aaa-bab7-4cee9a4df031\/stderr","size":255,"uid":"root"},{"gid":"root","mode":"-rw-r--r--","mtime":1496161812.0,"nlink":1,"path":"\/tmp\/slave\/slaves\/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-S0\/frameworks\/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-0001\/executors\/testapp-924b5c1524d84676a6a71665e2054d31\/runs\/cbec99c9-da2d-4aaa-bab7-4cee9a4df031\/stdout","size":1142,"uid":"root"}]
> ```
>
> Checked the new behavior (this would return a 404 before this patch):
> ```
> curl -i
> localhost:5051/files/browse?path=/tmp/slave/slaves/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-S0/frameworks/d3566e51-4d13-4eors/testapp-924b5c1524d84676a6a71665e2054d31/runs/latest
> HTTP/1.1 200 OK
> Date: Tue, 30 May 2017 17:43:13 GMT
> Content-Length: 584
> Content-Type: application/json
>
> [{"gid":"root","mode":"-rw-r--r--","mtime":1496161813.0,"nlink":1,"path":"\/tmp\/slave\/slaves\/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-S0\/frameworks\/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-0001\/executors\/testapp-924b5c1524d84676a6a71665e2054d31\/runs\/latest\/stderr","size":255,"uid":"root"},{"gid":"root","mode":"-rw-r--r--","mtime":1496161812.0,"nlink":1,"path":"\/tmp\/slave\/slaves\/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-S0\/frameworks\/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-0001\/executors\/testapp-924b5c1524d84676a6a71665e2054d31\/runs\/latest\/stdout","size":1142,"uid":"root"}]
> ```
>
>
> Thanks,
>
> Aaron Wood
>
>
Review Request 59641: Follow symlinks when resolving paths specified in the various master/agent endpoints.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59641/
---
Review request for mesos, Andrew Schwartzmeyer, James Peach, Vinod Kone, and
Zhitao Li.
Bugs: MESOS-7572
https://issues.apache.org/jira/browse/MESOS-7572
Repository: mesos
Description
---
The main benefit of following symlinks in endpoints such as `/files` is that
frameworks will be able to construct a path to the sandbox much easier. This
will assist framework developers in making features that need to provide a path
when hitting various operator API endpoints. Currently, making use of a path
ending in `runs/latest` throws a 404.
One such application could be a scheduler providing the ability for users to
work with their task's sandbox directly without going to the Mesos UI, API
endpoints, or the actual system themselves.
Diffs
-
src/files/files.cpp b03279ee0
Diff: https://reviews.apache.org/r/59641/diff/1/
Testing
---
`mkdir build && cd build && cmake .. && make -j2 && make check -j2`
Checked the original behavior:
```
curl -i
localhost:5051/files/browse?path=/tmp/slave/slaves/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-S0/frameworks/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-0001/executors/testapp-924b5c1524d84676a6a71665e2054d31/runs/cbec99c9-da2d-4aaa-bab7-4cee9a4df031
HTTP/1.1 200 OK
Date: Tue, 30 May 2017 17:43:08 GMT
Content-Length: 644
Content-Type: application/json
[{"gid":"root","mode":"-rw-r--r--","mtime":1496161813.0,"nlink":1,"path":"\/tmp\/slave\/slaves\/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-S0\/frameworks\/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-0001\/executors\/testapp-924b5c1524d84676a6a71665e2054d31\/runs\/cbec99c9-da2d-4aaa-bab7-4cee9a4df031\/stderr","size":255,"uid":"root"},{"gid":"root","mode":"-rw-r--r--","mtime":1496161812.0,"nlink":1,"path":"\/tmp\/slave\/slaves\/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-S0\/frameworks\/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-0001\/executors\/testapp-924b5c1524d84676a6a71665e2054d31\/runs\/cbec99c9-da2d-4aaa-bab7-4cee9a4df031\/stdout","size":1142,"uid":"root"}]
```
Checked the new behavior (this would return a 404 before this patch):
```
curl -i
localhost:5051/files/browse?path=/tmp/slave/slaves/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-S0/frameworks/d3566e51-4d13-4eors/testapp-924b5c1524d84676a6a71665e2054d31/runs/latest
HTTP/1.1 200 OK
Date: Tue, 30 May 2017 17:43:13 GMT
Content-Length: 584
Content-Type: application/json
[{"gid":"root","mode":"-rw-r--r--","mtime":1496161813.0,"nlink":1,"path":"\/tmp\/slave\/slaves\/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-S0\/frameworks\/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-0001\/executors\/testapp-924b5c1524d84676a6a71665e2054d31\/runs\/latest\/stderr","size":255,"uid":"root"},{"gid":"root","mode":"-rw-r--r--","mtime":1496161812.0,"nlink":1,"path":"\/tmp\/slave\/slaves\/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-S0\/frameworks\/d3566e51-4d13-4ed1-b66b-d538d3d7ef28-0001\/executors\/testapp-924b5c1524d84676a6a71665e2054d31\/runs\/latest\/stdout","size":1142,"uid":"root"}]
```
Thanks,
Aaron Wood
Re: Review Request 59413: Fix bytes.hpp constexpr compilation failure with GCC 7.1.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59413/ --- (Updated May 25, 2017, 6:13 p.m.) Review request for mesos, Benjamin Bannier, James Peach, Michael Park, and Neil Conway. Bugs: MESOS-7520 https://issues.apache.org/jira/browse/MESOS-7520 Repository: mesos Description --- Many of the `constexpr` variables fail to compile with errors such as `error: 'Megabytes(32).Megabytes::' is not a constant expression because it refers to an incompletely initialized variable`. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80829 This patch changes around the `Bytes` class a bit by getting rid of the small classes that extend `Bytes`. Diffs - 3rdparty/stout/include/stout/bytes.hpp 98223db50 Diff: https://reviews.apache.org/r/59413/diff/5/ Testing (updated) --- `./bootstrap && mkdir build && cd build && ../configure --disable-python --disable-java --disable-optimize --disable-hardening && make -j$(nproc) && make check -j$(nproc)` Thanks, Aaron Wood
Re: Review Request 59413: Fix bytes.hpp constexpr compilation failure with GCC 7.1.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59413/ --- (Updated May 25, 2017, 4:42 p.m.) Review request for mesos, Benjamin Bannier, James Peach, Michael Park, and Neil Conway. Changes --- Fixed formatting. Bugs: MESOS-7520 https://issues.apache.org/jira/browse/MESOS-7520 Repository: mesos Description --- Many of the `constexpr` variables fail to compile with errors such as `error: 'Megabytes(32).Megabytes::' is not a constant expression because it refers to an incompletely initialized variable`. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80829 This patch changes around the `Bytes` class a bit by getting rid of the small classes that extend `Bytes`. Diffs (updated) - 3rdparty/stout/include/stout/bytes.hpp 98223db50 Diff: https://reviews.apache.org/r/59413/diff/5/ Changes: https://reviews.apache.org/r/59413/diff/4-5/ Testing --- `./bootstrap && mkdir build && cd build && ../configure --disable-python --disable-java --disable-optimize --disable-hardening && make -j$(nproc)` Thanks, Aaron Wood
Re: Review Request 59536: CMake: Added SHA256 hashes for 3rdparty downloads.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59536/#review175985 --- Ship it! Ship It! - Aaron Wood On May 24, 2017, 7:13 p.m., Andrew Schwartzmeyer wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/59536/ > --- > > (Updated May 24, 2017, 7:13 p.m.) > > > Review request for mesos, Aaron Wood and Joseph Wu. > > > Repository: mesos > > > Description > --- > > The `ExternalProject_Add` uses this hash to verify the tarballs > automatically. This removes the following warning: > > File will not be verified since no URL_HASH specified > > > Diffs > - > > 3rdparty/CMakeLists.txt 03f314fdbb8216ca21abed3108e6986d037b2019 > 3rdparty/cmake/Versions.cmake d3f572c0b54fea4494fee033c549fa47c301 > > > Diff: https://reviews.apache.org/r/59536/diff/1/ > > > Testing > --- > > make check on Linux, cmake --build . --target mesos-tests and stout-tests on > Windows. > > > Thanks, > > Andrew Schwartzmeyer > >
Re: Review Request 59536: CMake: Added SHA256 hashes for 3rdparty downloads.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59536/#review175979 --- Are these hashes checked anywhere? I didn't seen anything that's verifying the hashes. - Aaron Wood On May 24, 2017, 7:13 p.m., Andrew Schwartzmeyer wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/59536/ > --- > > (Updated May 24, 2017, 7:13 p.m.) > > > Review request for mesos, Aaron Wood and Joseph Wu. > > > Repository: mesos > > > Description > --- > > The `ExternalProject_Add` uses this hash to verify the tarballs > automatically. This removes the following warning: > > File will not be verified since no URL_HASH specified > > > Diffs > - > > 3rdparty/CMakeLists.txt 03f314fdbb8216ca21abed3108e6986d037b2019 > 3rdparty/cmake/Versions.cmake d3f572c0b54fea4494fee033c549fa47c301 > > > Diff: https://reviews.apache.org/r/59536/diff/1/ > > > Testing > --- > > make check on Linux, cmake --build . --target mesos-tests and stout-tests on > Windows. > > > Thanks, > > Andrew Schwartzmeyer > >
Re: Review Request 59454: Adjust tests to account for GCC 7.1 fix in bytes.hpp.
> On May 23, 2017, 10:50 p.m., James Peach wrote: > > src/tests/role_tests.cpp > > Line 909 (original), 909 (patched) > > <https://reviews.apache.org/r/59454/diff/2/?file=1726951#file1726951line909> > > > > I think this should be: > > ``` > > constexpr Bytes DISK_SIZE = Megabytes(1); > > ``` Thanks for the catch. - Aaron --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59454/#review175875 --- On May 24, 2017, 5:53 p.m., Aaron Wood wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/59454/ > --- > > (Updated May 24, 2017, 5:53 p.m.) > > > Review request for mesos, Benjamin Bannier, James Peach, Michael Park, and > Neil Conway. > > > Bugs: MESOS-7520 > https://issues.apache.org/jira/browse/MESOS-7520 > > > Repository: mesos > > > Description > --- > > This fixes tests that uses the old inheritance types from `bytes.hpp`. > > > Diffs > - > > src/tests/persistent_volume_tests.cpp ef0085316 > src/tests/role_tests.cpp 03b6f7bd6 > > > Diff: https://reviews.apache.org/r/59454/diff/3/ > > > Testing > --- > > `./bootstrap && mkdir build && cd build && ../configure --disable-python > --disable-java --disable-optimize --disable-hardening && make check > -j$(nproc)` > > > Thanks, > > Aaron Wood > >
Re: Review Request 59454: Adjust tests to account for GCC 7.1 fix in bytes.hpp.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59454/ --- (Updated May 24, 2017, 5:53 p.m.) Review request for mesos, Benjamin Bannier, James Peach, Michael Park, and Neil Conway. Bugs: MESOS-7520 https://issues.apache.org/jira/browse/MESOS-7520 Repository: mesos Description --- This fixes tests that uses the old inheritance types from `bytes.hpp`. Diffs (updated) - src/tests/persistent_volume_tests.cpp ef0085316 src/tests/role_tests.cpp 03b6f7bd6 Diff: https://reviews.apache.org/r/59454/diff/3/ Changes: https://reviews.apache.org/r/59454/diff/2-3/ Testing --- `./bootstrap && mkdir build && cd build && ../configure --disable-python --disable-java --disable-optimize --disable-hardening && make check -j$(nproc)` Thanks, Aaron Wood
Review Request 59454: Adjust test to account for changes to bytes.hpp.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59454/ --- Review request for mesos, Benjamin Bannier, James Peach, Michael Park, and Neil Conway. Bugs: MESOS-7520 https://issues.apache.org/jira/browse/MESOS-7520 Repository: mesos Description --- This fixes a test that uses the old inheritance types from `bytes.hpp`. Diffs - src/tests/persistent_volume_tests.cpp ef0085316 Diff: https://reviews.apache.org/r/59454/diff/1/ Testing --- `./bootstrap && mkdir build && cd build && ../configure --disable-python --disable-java --disable-optimize --disable-hardening && make check -j$(nproc)` Thanks, Aaron Wood
Re: Review Request 59413: Fix bytes.hpp constexpr compilation failure with GCC 7.1.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59413/ --- (Updated May 22, 2017, 4:56 p.m.) Review request for mesos, Benjamin Bannier, James Peach, Michael Park, and Neil Conway. Bugs: MESOS-7520 https://issues.apache.org/jira/browse/MESOS-7520 Repository: mesos Description --- Many of the `constexpr` variables fail to compile with errors such as `error: 'Megabytes(32).Megabytes::' is not a constant expression because it refers to an incompletely initialized variable`. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80829 This patch changes around the `Bytes` class a bit by getting rid of the small classes that extend `Bytes`. Diffs - 3rdparty/stout/include/stout/bytes.hpp 98223db50 Diff: https://reviews.apache.org/r/59413/diff/4/ Testing --- `./bootstrap && mkdir build && cd build && ../configure --disable-python --disable-java --disable-optimize --disable-hardening && make -j$(nproc)` Thanks, Aaron Wood
Re: Review Request 59413: Fix bytes.hpp constexpr compilation failure with GCC 7.1.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59413/ --- (Updated May 22, 2017, 4:52 p.m.) Review request for mesos, Benjamin Bannier, James Peach, Michael Park, and Neil Conway. Changes --- Rework patch to only target bytes.hpp. Summary (updated) - Fix bytes.hpp constexpr compilation failure with GCC 7.1. Bugs: MESOS-7520 https://issues.apache.org/jira/browse/MESOS-7520 Repository: mesos Description (updated) --- Many of the `constexpr` variables fail to compile with errors such as `error: 'Megabytes(32).Megabytes::' is not a constant expression because it refers to an incompletely initialized variable`. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80829 This patch changes around the `Bytes` class a bit by getting rid of the small classes that extend `Bytes`. Diffs (updated) - 3rdparty/stout/include/stout/bytes.hpp 98223db50 Diff: https://reviews.apache.org/r/59413/diff/4/ Changes: https://reviews.apache.org/r/59413/diff/3-4/ Testing --- `./bootstrap && mkdir build && cd build && ../configure --disable-python --disable-java --disable-optimize --disable-hardening && make -j$(nproc)` Thanks, Aaron Wood
Re: Review Request 59413: Fix constexpr compilation failure with GCC 7.1.
> On May 19, 2017, 6:51 p.m., Neil Conway wrote:
> > `stout`, `libprocess`, and `mesos` are nominally distinct projects, so a
> > single RR should touch no more than one of them. i.e., please split the
> > `stout` changes into a separate review.
> >
> > Might also be worth waiting to see if GCC upstream is going to fix the
> > problem promptly. If GCC 7.2 fixes the problem, we don't necessarily need
> > to workaround a bug that occurs in just a single minor release of GCC.
>
> Aaron Wood wrote:
> Sure, I can hold on this and see what they say. If it's something that
> still needs to be fixed I'll break out this RR into separate ones.
>
> Benjamin Bannier wrote:
> I am not a big fan of the derived classes of `Bytes` since they are
> practically designed to be used with slicing (`Bytes b = Kilobytes(42)`)
> which is nasty by itself. I believe the intention here was to provide easy to
> use factory functions, and don't believe introducing dedicated types for
> multiples is a very useful but instead confusing (think e.g., number of
> implicit conversions and overload resolution). This fix makes sense
> regardless of whether GCC fixes this particular regression.
I think it is much cleaner as well. If everyone is in agreement I can move
forward with this patch regardless of the GCC issue.
> On May 19, 2017, 6:51 p.m., Neil Conway wrote:
> > src/master/constants.hpp
> > Line 49 (original), 49 (patched)
> > <https://reviews.apache.org/r/59413/diff/1/?file=1725380#file1725380line49>
> >
> > This change (and all the similar changes) seems unfortunate. Can't we
> > play a similar trick to the change you made to `Bytes`?
>
> Aaron Wood wrote:
> I had wanted to but I didn't see a quick and easy way to do this without
> making some major changes. The extended classes in `duration.hpp` mostly look
> something like this:
> ```
> class Nanoseconds : public Duration
> {
> public:
> explicit constexpr Nanoseconds(int64_t nanoseconds)
> : Duration(nanoseconds, NANOSECONDS) {}
>
> constexpr Nanoseconds(const Duration& d) : Duration(d) {}
>
> double value() const { return static_cast(this->ns()); }
>
> static std::string units() { return "ns"; }
> };
> ```
> I'm open to ideas, I just figured if I changed these clases a lot I'd
> have even more code to change around Mesos.
>
> Benjamin Bannier wrote:
> It looks like the only custom method here is `units` which seems to be
> used by just the lengthy stringification function below. I believe getting
> rid of the derived classes here makes just as much sense as it did for
> `Bytes` and friends.
That's good to know, I didn't dig too deeply to see what was using some of this
stuff. I can rework this file a bit so that we could keep the `Duration` base
type everywhere like it was.
- Aaron
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59413/#review17
---
On May 19, 2017, 7 p.m., Aaron Wood wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59413/
> ---
>
> (Updated May 19, 2017, 7 p.m.)
>
>
> Review request for mesos, Benjamin Bannier, James Peach, Michael Park, and
> Neil Conway.
>
>
> Bugs: MESOS-7520
> https://issues.apache.org/jira/browse/MESOS-7520
>
>
> Repository: mesos
>
>
> Description
> ---
>
> Many of the `constexpr` variables fail to compile with errors such as `error:
> 'Megabytes(32).Megabytes::' is not a constant expression because
> it refers to an incompletely initialized variable`.
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80829
>
> This patch changes around the `Bytes` class a bit by getting rid of the small
> classes that extend `Bytes`. Additionally, any usage of the `Duration` class
> has been adjusted to not instantiate using the base type (which triggers the
> issue along with `constexpr`).
>
>
> Diffs
> -
>
> 3rdparty/stout/include/stout/bytes.hpp 98223db50
> src/master/constants.hpp 725680b1e
> src/sched/constants.hpp 9edb25b38
> src/sched/sched.cpp ef73c1dcc
> src/scheduler/constants.hpp e3da12646
> src/slave/constants.hpp 9c1d7245c
> src/slave/containerizer/mesos/isolators/docker/volume/driver.cpp 929b7e0b7
> src/slave/slave.cpp 14de72fa4
> src/slave/status_update_manager.cpp 0cd88ac3a
>
>
> Diff: https://reviews.apache.org/r/59413/diff/3/
>
>
> Testing
> ---
>
> `./bootstrap && mkdir build && cd build && ../configure --disable-python
> --disable-java --disable-optimize --disable-hardening && make -j$(nproc)`
>
>
> Thanks,
>
> Aaron Wood
>
>
Re: Review Request 59413: Fix constexpr compilation failure with GCC 7.1.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59413/ --- (Updated May 19, 2017, 7 p.m.) Review request for mesos, Benjamin Bannier, James Peach, Michael Park, and Neil Conway. Changes --- Reference filed GCC bug. Bugs: MESOS-7520 https://issues.apache.org/jira/browse/MESOS-7520 Repository: mesos Description (updated) --- Many of the `constexpr` variables fail to compile with errors such as `error: 'Megabytes(32).Megabytes::' is not a constant expression because it refers to an incompletely initialized variable`. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80829 This patch changes around the `Bytes` class a bit by getting rid of the small classes that extend `Bytes`. Additionally, any usage of the `Duration` class has been adjusted to not instantiate using the base type (which triggers the issue along with `constexpr`). Diffs - 3rdparty/stout/include/stout/bytes.hpp 98223db50 src/master/constants.hpp 725680b1e src/sched/constants.hpp 9edb25b38 src/sched/sched.cpp ef73c1dcc src/scheduler/constants.hpp e3da12646 src/slave/constants.hpp 9c1d7245c src/slave/containerizer/mesos/isolators/docker/volume/driver.cpp 929b7e0b7 src/slave/slave.cpp 14de72fa4 src/slave/status_update_manager.cpp 0cd88ac3a Diff: https://reviews.apache.org/r/59413/diff/3/ Testing --- `./bootstrap && mkdir build && cd build && ../configure --disable-python --disable-java --disable-optimize --disable-hardening && make -j$(nproc)` Thanks, Aaron Wood
Re: Review Request 59413: Fix constexpr compilation failure with GCC 7.1.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59413/ --- (Updated May 19, 2017, 6:58 p.m.) Review request for mesos, Benjamin Bannier, James Peach, Michael Park, and Neil Conway. Changes --- Clean base for patch to be applied. Bugs: MESOS-7520 https://issues.apache.org/jira/browse/MESOS-7520 Repository: mesos Description --- Many of the `constexpr` variables fail to compile with errors such as `error: 'Megabytes(32).Megabytes::' is not a constant expression because it refers to an incompletely initialized variable`. This patch changes around the `Bytes` class a bit by getting rid of the small classes that extend `Bytes`. Additionally, any usage of the `Duration` class has been adjusted to not instantiate using the base type (which triggers the issue along with `constexpr`). Diffs (updated) - 3rdparty/stout/include/stout/bytes.hpp 98223db50 src/master/constants.hpp 725680b1e src/sched/constants.hpp 9edb25b38 src/sched/sched.cpp ef73c1dcc src/scheduler/constants.hpp e3da12646 src/slave/constants.hpp 9c1d7245c src/slave/containerizer/mesos/isolators/docker/volume/driver.cpp 929b7e0b7 src/slave/slave.cpp 14de72fa4 src/slave/status_update_manager.cpp 0cd88ac3a Diff: https://reviews.apache.org/r/59413/diff/3/ Changes: https://reviews.apache.org/r/59413/diff/2-3/ Testing --- `./bootstrap && mkdir build && cd build && ../configure --disable-python --disable-java --disable-optimize --disable-hardening && make -j$(nproc)` Thanks, Aaron Wood
Re: Review Request 59413: Fix constexpr compilation failure with GCC 7.1.
> On May 19, 2017, 6:51 p.m., Neil Conway wrote:
> > `stout`, `libprocess`, and `mesos` are nominally distinct projects, so a
> > single RR should touch no more than one of them. i.e., please split the
> > `stout` changes into a separate review.
> >
> > Might also be worth waiting to see if GCC upstream is going to fix the
> > problem promptly. If GCC 7.2 fixes the problem, we don't necessarily need
> > to workaround a bug that occurs in just a single minor release of GCC.
Sure, I can hold on this and see what they say. If it's something that still
needs to be fixed I'll break out this RR into separate ones.
> On May 19, 2017, 6:51 p.m., Neil Conway wrote:
> > src/master/constants.hpp
> > Line 49 (original), 49 (patched)
> > <https://reviews.apache.org/r/59413/diff/1/?file=1725380#file1725380line49>
> >
> > This change (and all the similar changes) seems unfortunate. Can't we
> > play a similar trick to the change you made to `Bytes`?
I had wanted to but I didn't see a quick and easy way to do this without making
some major changes. The extended classes in `duration.hpp` mostly look
something like this:
```
class Nanoseconds : public Duration
{
public:
explicit constexpr Nanoseconds(int64_t nanoseconds)
: Duration(nanoseconds, NANOSECONDS) {}
constexpr Nanoseconds(const Duration& d) : Duration(d) {}
double value() const { return static_cast(this->ns()); }
static std::string units() { return "ns"; }
};
```
I'm open to ideas, I just figured if I changed these clases a lot I'd have even
more code to change around Mesos.
- Aaron
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59413/#review17
---
On May 19, 2017, 6:51 p.m., Aaron Wood wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59413/
> ---
>
> (Updated May 19, 2017, 6:51 p.m.)
>
>
> Review request for mesos, Benjamin Bannier, James Peach, Michael Park, and
> Neil Conway.
>
>
> Bugs: MESOS-7520
> https://issues.apache.org/jira/browse/MESOS-7520
>
>
> Repository: mesos
>
>
> Description
> ---
>
> Many of the `constexpr` variables fail to compile with errors such as `error:
> 'Megabytes(32).Megabytes::' is not a constant expression because
> it refers to an incompletely initialized variable`.
>
> This patch changes around the `Bytes` class a bit by getting rid of the small
> classes that extend `Bytes`. Additionally, any usage of the `Duration` class
> has been adjusted to not instantiate using the base type (which triggers the
> issue along with `constexpr`).
>
>
> Diffs
> -
>
> 3rdparty/stout/include/stout/bytes.hpp 98223db50
> src/master/constants.hpp 7edf9f65f
> src/sched/constants.hpp 9edb25b38
> src/sched/sched.cpp ef73c1dcc
> src/scheduler/constants.hpp e3da12646
> src/slave/constants.hpp 1159ac3b1
> src/slave/containerizer/mesos/isolators/docker/volume/driver.cpp cf0466c62
> src/slave/slave.cpp 7564e8d39
> src/slave/status_update_manager.cpp df63a708c
>
>
> Diff: https://reviews.apache.org/r/59413/diff/2/
>
>
> Testing
> ---
>
> `./bootstrap && mkdir build && cd build && ../configure --disable-python
> --disable-java --disable-optimize --disable-hardening && make -j$(nproc)`
>
>
> Thanks,
>
> Aaron Wood
>
>
Re: Review Request 59413: Fix constexpr compilation failure with GCC 7.1.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59413/ --- (Updated May 19, 2017, 6:51 p.m.) Review request for mesos, Benjamin Bannier, James Peach, Michael Park, and Neil Conway. Changes --- Rebased on the latest HEAD. Bugs: MESOS-7520 https://issues.apache.org/jira/browse/MESOS-7520 Repository: mesos Description --- Many of the `constexpr` variables fail to compile with errors such as `error: 'Megabytes(32).Megabytes::' is not a constant expression because it refers to an incompletely initialized variable`. This patch changes around the `Bytes` class a bit by getting rid of the small classes that extend `Bytes`. Additionally, any usage of the `Duration` class has been adjusted to not instantiate using the base type (which triggers the issue along with `constexpr`). Diffs (updated) - 3rdparty/stout/include/stout/bytes.hpp 98223db50 src/master/constants.hpp 7edf9f65f src/sched/constants.hpp 9edb25b38 src/sched/sched.cpp ef73c1dcc src/scheduler/constants.hpp e3da12646 src/slave/constants.hpp 1159ac3b1 src/slave/containerizer/mesos/isolators/docker/volume/driver.cpp cf0466c62 src/slave/slave.cpp 7564e8d39 src/slave/status_update_manager.cpp df63a708c Diff: https://reviews.apache.org/r/59413/diff/2/ Changes: https://reviews.apache.org/r/59413/diff/1-2/ Testing --- `./bootstrap && mkdir build && cd build && ../configure --disable-python --disable-java --disable-optimize --disable-hardening && make -j$(nproc)` Thanks, Aaron Wood
Review Request 59413: Fix constexpr compilation failure with GCC 7.1.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59413/ --- Review request for mesos, Benjamin Bannier, James Peach, Michael Park, and Neil Conway. Bugs: MESOS-7520 https://issues.apache.org/jira/browse/MESOS-7520 Repository: mesos Description --- Many of the `constexpr` variables fail to compile with errors such as `error: 'Megabytes(32).Megabytes::' is not a constant expression because it refers to an incompletely initialized variable`. This patch changes around the `Bytes` class a bit by getting rid of the small classes that extend `Bytes`. Additionally, any usage of the `Duration` class has been adjusted to not instantiate using the base type (which triggers the issue along with `constexpr`). Diffs - 3rdparty/stout/include/stout/bytes.hpp 98223db50 src/master/constants.hpp 7edf9f65f src/sched/constants.hpp 9edb25b38 src/sched/sched.cpp ef73c1dcc src/scheduler/constants.hpp e3da12646 src/slave/constants.hpp 1159ac3b1 src/slave/containerizer/mesos/isolators/docker/volume/driver.cpp cf0466c62 src/slave/slave.cpp 7564e8d39 src/slave/status_update_manager.cpp df63a708c Diff: https://reviews.apache.org/r/59413/diff/1/ Testing --- `./bootstrap && mkdir build && cd build && ../configure --disable-python --disable-java --disable-optimize --disable-hardening && make -j$(nproc)` Thanks, Aaron Wood
Re: Review Request 58317: Prevent agent from crashing when the ID is longer than the maximum file name length.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58317/ --- (Updated April 11, 2017, 3:14 p.m.) Review request for mesos, Jie Yu and James Peach. Changes --- Use `NAME_MAX` in the error message and reorder C header. Bugs: MESOS-7346 https://issues.apache.org/jira/browse/MESOS-7346 Repository: mesos Description --- When launching a task with a very long name `mkdir` will fail and crash the agent, making it quick and easy to remotely crash any agent(s). This changes makes sure that we never exceed the default file name limit of most of the file systems out there. If the limit is exceed a `TASK_ERROR` is instead sent back to the client. Diffs (updated) - src/common/validation.cpp 544d3a0bf Diff: https://reviews.apache.org/r/58317/diff/3/ Changes: https://reviews.apache.org/r/58317/diff/2-3/ Testing --- Built Mesos, ran 1 master and 1 agent locally, and launched a task with a name > 255 characters through our own custom framework. ``` I0410 14:04:15.761109 28528 master.cpp:6323] Sending status update TASK_ERROR for task tesijetjijwtjwitjweitjweitjwitjweitjtijeiojwetoijetweoijwilgjwkdgsjskd.nlskdgnldksnglsdkgndslgjdslgjsdlgjldskgjsdlkgjslkgjslkdgjsdlkgjskldgjweiojgoeiwgjweoigjewoitjweoitjewiotjewitjweitjewitjwewejiowejiewtiwethewothweuothewouthuthwouthwuothgwuoghwouhweouthewhtiewtwetwhtohwethwetwehtwehtwjethwejlthwejlthwejhtwejthwejkthwejkthwejkthwejthwkthwekthwtkjwthkwjehtjkwehtkjwehtkjwehtkjwehtkwejhtewjkthewkjthewjkthwekjthwekjthwekjthwekjthwekjhtwekhtewkapplication227741021670-4A491BFF-F50D-4F17-B304-AD23423F15EE of framework 9e8d9439-7ed2-4968-a8cb-f46273c2d9af- 'ID must not be greater than 255 characters' ``` Thanks, Aaron Wood
Re: Review Request 58317: Prevent agent from crashing when the ID is longer than the maximum file name length.
> On April 10, 2017, 11:48 p.m., Jie Yu wrote: > > src/common/validation.cpp > > Lines 42 (patched) > > <https://reviews.apache.org/r/58317/diff/2/?file=1687727#file1687727line42> > > > > 255? Cau you use NAME_MAX in the error message as well? My fault, I should have done that initially when I switched to using `NAME_MAX`. - Aaron --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58317/#review171505 --- On April 10, 2017, 10:37 p.m., Aaron Wood wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/58317/ > --- > > (Updated April 10, 2017, 10:37 p.m.) > > > Review request for mesos, Jie Yu and James Peach. > > > Bugs: MESOS-7346 > https://issues.apache.org/jira/browse/MESOS-7346 > > > Repository: mesos > > > Description > --- > > When launching a task with a very long name `mkdir` will fail and crash the > agent, making it quick and easy to remotely crash any agent(s). This changes > makes sure that we never exceed the default file name limit of most of the > file systems out there. If the limit is exceed a `TASK_ERROR` is instead sent > back to the client. > > > Diffs > - > > src/common/validation.cpp 544d3a0bf > > > Diff: https://reviews.apache.org/r/58317/diff/2/ > > > Testing > --- > > Built Mesos, ran 1 master and 1 agent locally, and launched a task with a > name > 255 characters through our own custom framework. > > ``` > I0410 14:04:15.761109 28528 master.cpp:6323] Sending status update TASK_ERROR > for task > tesijetjijwtjwitjweitjweitjwitjweitjtijeiojwetoijetweoijwilgjwkdgsjskd.nlskdgnldksnglsdkgndslgjdslgjsdlgjldskgjsdlkgjslkgjslkdgjsdlkgjskldgjweiojgoeiwgjweoigjewoitjweoitjewiotjewitjweitjewitjwewejiowejiewtiwethewothweuothewouthuthwouthwuothgwuoghwouhweouthewhtiewtwetwhtohwethwetwehtwehtwjethwejlthwejlthwejhtwejthwejkthwejkthwejkthwejthwkthwekthwtkjwthkwjehtjkwehtkjwehtkjwehtkjwehtkwejhtewjkthewkjthewjkthwekjthwekjthwekjthwekjthwekjhtwekhtewkapplication227741021670-4A491BFF-F50D-4F17-B304-AD23423F15EE > of framework 9e8d9439-7ed2-4968-a8cb-f46273c2d9af- 'ID must not be > greater than 255 characters' > ``` > > > Thanks, > > Aaron Wood > >
Re: Review Request 58317: Prevent agent from crashing when the ID is longer than the maximum file name length.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58317/ --- (Updated April 10, 2017, 10:37 p.m.) Review request for mesos, Jie Yu and James Peach. Changes --- Use `NAME_MAX` instead of a hardcoded number. Bugs: MESOS-7346 https://issues.apache.org/jira/browse/MESOS-7346 Repository: mesos Description --- When launching a task with a very long name `mkdir` will fail and crash the agent, making it quick and easy to remotely crash any agent(s). This changes makes sure that we never exceed the default file name limit of most of the file systems out there. If the limit is exceed a `TASK_ERROR` is instead sent back to the client. Diffs (updated) - src/common/validation.cpp 544d3a0bf Diff: https://reviews.apache.org/r/58317/diff/2/ Changes: https://reviews.apache.org/r/58317/diff/1-2/ Testing --- Built Mesos, ran 1 master and 1 agent locally, and launched a task with a name > 255 characters through our own custom framework. ``` I0410 14:04:15.761109 28528 master.cpp:6323] Sending status update TASK_ERROR for task tesijetjijwtjwitjweitjweitjwitjweitjtijeiojwetoijetweoijwilgjwkdgsjskd.nlskdgnldksnglsdkgndslgjdslgjsdlgjldskgjsdlkgjslkgjslkdgjsdlkgjskldgjweiojgoeiwgjweoigjewoitjweoitjewiotjewitjweitjewitjwewejiowejiewtiwethewothweuothewouthuthwouthwuothgwuoghwouhweouthewhtiewtwetwhtohwethwetwehtwehtwjethwejlthwejlthwejhtwejthwejkthwejkthwejkthwejthwkthwekthwtkjwthkwjehtjkwehtkjwehtkjwehtkjwehtkwejhtewjkthewkjthewjkthwekjthwekjthwekjthwekjthwekjhtwekhtewkapplication227741021670-4A491BFF-F50D-4F17-B304-AD23423F15EE of framework 9e8d9439-7ed2-4968-a8cb-f46273c2d9af- 'ID must not be greater than 255 characters' ``` Thanks, Aaron Wood
Re: Review Request 58317: Prevent agent from crashing when the ID is longer than the maximum file name length.
> On April 10, 2017, 9:05 p.m., Aaron Wood wrote: > > src/common/validation.cpp > > Lines 40 (patched) > > <https://reviews.apache.org/r/58317/diff/1/?file=1687524#file1687524line40> > > > > Maybe it's better to put this check in `src/slave/paths.cpp` and add up > > the length of `rootDir`, `slaveId`, `frameworkId`, `executorId`, and > > `containerId`? What I have here doesn't really guard against the entire > > path being > 255, only individual pieces. > > > > I could add this check around here: > > ``` > > const string directory = > > getExecutorRunPath(rootDir, slaveId, frameworkId, executorId, > > containerId); > > > > Try mkdir = os::mkdir(directory); > > ``` > > > > Thoughts? > > James Peach wrote: > The path is allowed to be > 255 (typically 4K on Linux). Ah, you're right. Looks like this check should be good enough then! - Aaron --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58317/#review171483 --- On April 10, 2017, 6:16 p.m., Aaron Wood wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/58317/ > --- > > (Updated April 10, 2017, 6:16 p.m.) > > > Review request for mesos, Jie Yu and James Peach. > > > Bugs: MESOS-7346 > https://issues.apache.org/jira/browse/MESOS-7346 > > > Repository: mesos > > > Description > --- > > When launching a task with a very long name `mkdir` will fail and crash the > agent, making it quick and easy to remotely crash any agent(s). This changes > makes sure that we never exceed the default file name limit of most of the > file systems out there. If the limit is exceed a `TASK_ERROR` is instead sent > back to the client. > > > Diffs > - > > src/common/validation.cpp 544d3a0bf > > > Diff: https://reviews.apache.org/r/58317/diff/1/ > > > Testing > --- > > Built Mesos, ran 1 master and 1 agent locally, and launched a task with a > name > 255 characters through our own custom framework. > > ``` > I0410 14:04:15.761109 28528 master.cpp:6323] Sending status update TASK_ERROR > for task > tesijetjijwtjwitjweitjweitjwitjweitjtijeiojwetoijetweoijwilgjwkdgsjskd.nlskdgnldksnglsdkgndslgjdslgjsdlgjldskgjsdlkgjslkgjslkdgjsdlkgjskldgjweiojgoeiwgjweoigjewoitjweoitjewiotjewitjweitjewitjwewejiowejiewtiwethewothweuothewouthuthwouthwuothgwuoghwouhweouthewhtiewtwetwhtohwethwetwehtwehtwjethwejlthwejlthwejhtwejthwejkthwejkthwejkthwejthwkthwekthwtkjwthkwjehtjkwehtkjwehtkjwehtkjwehtkwejhtewjkthewkjthewjkthwekjthwekjthwekjthwekjthwekjhtwekhtewkapplication227741021670-4A491BFF-F50D-4F17-B304-AD23423F15EE > of framework 9e8d9439-7ed2-4968-a8cb-f46273c2d9af- 'ID must not be > greater than 255 characters' > ``` > > > Thanks, > > Aaron Wood > >
Re: Review Request 58317: Prevent agent from crashing when the ID is longer than the maximum file name length.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58317/#review171483 --- src/common/validation.cpp Lines 40 (patched) <https://reviews.apache.org/r/58317/#comment244418> Maybe it's better to put this check in `src/slave/paths.cpp` and add up the length of `rootDir`, `slaveId`, `frameworkId`, `executorId`, and `containerId`? What I have here doesn't really guard against the entire path being > 255, only individual pieces. I could add this check around here: ``` const string directory = getExecutorRunPath(rootDir, slaveId, frameworkId, executorId, containerId); Try mkdir = os::mkdir(directory); ``` Thoughts? - Aaron Wood On April 10, 2017, 6:16 p.m., Aaron Wood wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/58317/ > --- > > (Updated April 10, 2017, 6:16 p.m.) > > > Review request for mesos, Jie Yu and James Peach. > > > Bugs: MESOS-7346 > https://issues.apache.org/jira/browse/MESOS-7346 > > > Repository: mesos > > > Description > --- > > When launching a task with a very long name `mkdir` will fail and crash the > agent, making it quick and easy to remotely crash any agent(s). This changes > makes sure that we never exceed the default file name limit of most of the > file systems out there. If the limit is exceed a `TASK_ERROR` is instead sent > back to the client. > > > Diffs > - > > src/common/validation.cpp 544d3a0bf > > > Diff: https://reviews.apache.org/r/58317/diff/1/ > > > Testing > --- > > Built Mesos, ran 1 master and 1 agent locally, and launched a task with a > name > 255 characters through our own custom framework. > > ``` > I0410 14:04:15.761109 28528 master.cpp:6323] Sending status update TASK_ERROR > for task > tesijetjijwtjwitjweitjweitjwitjweitjtijeiojwetoijetweoijwilgjwkdgsjskd.nlskdgnldksnglsdkgndslgjdslgjsdlgjldskgjsdlkgjslkgjslkdgjsdlkgjskldgjweiojgoeiwgjweoigjewoitjweoitjewiotjewitjweitjewitjwewejiowejiewtiwethewothweuothewouthuthwouthwuothgwuoghwouhweouthewhtiewtwetwhtohwethwetwehtwehtwjethwejlthwejlthwejhtwejthwejkthwejkthwejkthwejthwkthwekthwtkjwthkwjehtjkwehtkjwehtkjwehtkjwehtkwejhtewjkthewkjthewjkthwekjthwekjthwekjthwekjthwekjhtwekhtewkapplication227741021670-4A491BFF-F50D-4F17-B304-AD23423F15EE > of framework 9e8d9439-7ed2-4968-a8cb-f46273c2d9af- 'ID must not be > greater than 255 characters' > ``` > > > Thanks, > > Aaron Wood > >
Re: Review Request 58317: Prevent agent from crashing when the ID is longer than the maximum file name length.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58317/#review171456 --- src/common/validation.cpp Line 46 (original), 50 (patched) <https://reviews.apache.org/r/58317/#comment244396> I figured I'd fix this spelling error while I was here. - Aaron Wood On April 10, 2017, 6:16 p.m., Aaron Wood wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/58317/ > --- > > (Updated April 10, 2017, 6:16 p.m.) > > > Review request for mesos, Jie Yu and James Peach. > > > Bugs: MESOS-7346 > https://issues.apache.org/jira/browse/MESOS-7346 > > > Repository: mesos > > > Description > --- > > When launching a task with a very long name `mkdir` will fail and crash the > agent, making it quick and easy to remotely crash any agent(s). This changes > makes sure that we never exceed the default file name limit of most of the > file systems out there. If the limit is exceed a `TASK_ERROR` is instead sent > back to the client. > > > Diffs > - > > src/common/validation.cpp 544d3a0bf > > > Diff: https://reviews.apache.org/r/58317/diff/1/ > > > Testing > --- > > Built Mesos, ran 1 master and 1 agent locally, and launched a task with a > name > 255 characters through our own custom framework. > > ``` > I0410 14:04:15.761109 28528 master.cpp:6323] Sending status update TASK_ERROR > for task > tesijetjijwtjwitjweitjweitjwitjweitjtijeiojwetoijetweoijwilgjwkdgsjskd.nlskdgnldksnglsdkgndslgjdslgjsdlgjldskgjsdlkgjslkgjslkdgjsdlkgjskldgjweiojgoeiwgjweoigjewoitjweoitjewiotjewitjweitjewitjwewejiowejiewtiwethewothweuothewouthuthwouthwuothgwuoghwouhweouthewhtiewtwetwhtohwethwetwehtwehtwjethwejlthwejlthwejhtwejthwejkthwejkthwejkthwejthwkthwekthwtkjwthkwjehtjkwehtkjwehtkjwehtkjwehtkwejhtewjkthewkjthewjkthwekjthwekjthwekjthwekjthwekjhtwekhtewkapplication227741021670-4A491BFF-F50D-4F17-B304-AD23423F15EE > of framework 9e8d9439-7ed2-4968-a8cb-f46273c2d9af- 'ID must not be > greater than 255 characters' > ``` > > > Thanks, > > Aaron Wood > >
Review Request 58317: Prevent agent from crashing when the ID is longer than the maximum file name length.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58317/ --- Review request for mesos, Jie Yu and James Peach. Bugs: MESOS-7346 https://issues.apache.org/jira/browse/MESOS-7346 Repository: mesos Description --- When launching a task with a very long name `mkdir` will fail and crash the agent, making it quick and easy to remotely crash any agent(s). This changes makes sure that we never exceed the default file name limit of most of the file systems out there. If the limit is exceed a `TASK_ERROR` is instead sent back to the client. Diffs - src/common/validation.cpp 544d3a0bf Diff: https://reviews.apache.org/r/58317/diff/1/ Testing --- Built Mesos, ran 1 master and 1 agent locally, and launched a task with a name > 255 characters through our own custom framework. ``` I0410 14:04:15.761109 28528 master.cpp:6323] Sending status update TASK_ERROR for task tesijetjijwtjwitjweitjweitjwitjweitjtijeiojwetoijetweoijwilgjwkdgsjskd.nlskdgnldksnglsdkgndslgjdslgjsdlgjldskgjsdlkgjslkgjslkdgjsdlkgjskldgjweiojgoeiwgjweoigjewoitjweoitjewiotjewitjweitjewitjwewejiowejiewtiwethewothweuothewouthuthwouthwuothgwuoghwouhweouthewhtiewtwetwhtohwethwetwehtwehtwjethwejlthwejlthwejhtwejthwejkthwejkthwejkthwejthwkthwekthwtkjwthkwjehtjkwehtkjwehtkjwehtkjwehtkwejhtewjkthewkjthewjkthwekjthwekjthwekjthwekjthwekjhtwekhtewkapplication227741021670-4A491BFF-F50D-4F17-B304-AD23423F15EE of framework 9e8d9439-7ed2-4968-a8cb-f46273c2d9af- 'ID must not be greater than 255 characters' ``` Thanks, Aaron Wood
Re: Review Request 54993: Modify LevelDB patch to add ARM64/AArch64 support.
> On Dec. 27, 2016, 1:48 p.m., Till Toenshoff wrote: > > This sums up the following changes on `atomic_pointer.h` done within the > > upstream project; > > https://github.com/google/leveldb/commit/803d69203a62faf50f1b77897310a3a1fcae712b > > https://github.com/google/leveldb/commit/c4c38f9c1f3bb405fe22a79c5611438f91208d09 > > https://github.com/google/leveldb/commit/ceff6f12152785a54885a47db349a6d8dfd0ce2c > > > > Note that some of those changes are unrelated to ARM64 and hence not > > strictly within the scope > > of this ticket -- so we do not need those changes. However, having things > > in sync with upstream > > is definitely valuable and in these specific cases appears to be free of > > additional risks. Should we close this now that LevelDB has been upgraded to 1.19? - Aaron --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54993/#review160160 --- On Dec. 22, 2016, 9:10 p.m., Aaron Wood wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/54993/ > --- > > (Updated Dec. 22, 2016, 9:10 p.m.) > > > Review request for mesos and Jie Yu. > > > Bugs: MESOS-6834 > https://issues.apache.org/jira/browse/MESOS-6834 > > > Repository: mesos > > > Description > --- > > Mesos will not compile on ARM64/AArch64 without a patch to the version of > LevelDB that is used within Mesos. While the fix is already in newer versions > of LevelDB it's not something that Mesos pulls down. > > The main issue is that the AtomicPointer header needs to be aware of other > architectures to provide its functionality to those architectures. > > > Diffs > - > > 3rdparty/leveldb-1.4.patch b899f0141 > > > Diff: https://reviews.apache.org/r/54993/diff/1/ > > > Testing > --- > > Compiled Mesos on ARM64 with no failures. Mesos also properly starts and > successfully runs/launches tasks with the exception of a crash when using the > linux_launcher and Mesos containers. That fix is addressed in > https://reviews.apache.org/r/54996/ > > > Thanks, > > Aaron Wood > >
Re: Review Request 55480: Fix segfault when the executor sets a UUID that is not a valid v4 UUID.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55480/ --- (Updated Jan. 16, 2017, 8:47 p.m.) Review request for mesos and Anand Mazumdar. Changes --- Alter test. Bugs: MESOS-6917 https://issues.apache.org/jira/browse/MESOS-6917 Repository: mesos Description --- This fixes the segfault that occurs when an executor sets a UUID that's not a valid v4 UUID and sends it off to the agent: ``` ABORT: (../../3rdparty/stout/include/stout/try.hpp:77): Try::get() but state == ERROR: Not a valid UUID *** Aborted at 1484262968 (unix time) try "date -d @1484262968" if you are using GNU date *** PC: @ 0x7efeb6101428 (unknown) *** SIGABRT (@0x36b7) received by PID 14007 (TID 0x7efeabd29700) from PID 14007; stack trace: *** @ 0x7efeb64a6390 (unknown) @ 0x7efeb6101428 (unknown) @ 0x7efeb610302a (unknown) @ 0x560df739fa6e _Abort() @ 0x560df739fa9c _Abort() @ 0x7efebb53a5ad Try<>::get() @ 0x7efebb5363d6 Try<>::get() @ 0x7efebbd84809 mesos::internal::slave::validation::executor::call::validate() @ 0x7efebbb59b36 mesos::internal::slave::Slave::Http::executor() @ 0x7efebbc773b8 _ZZN5mesos8internal5slave5Slave10initializeEvENKUlRKN7process4http7RequestEE1_clES7_ @ 0x7efebbcb5808 _ZNSt17_Function_handlerIFN7process6FutureINS0_4http8ResponseEEERKNS2_7RequestEEZN5mesos8internal5slave5Slave10initializeEvEUlS7_E1_E9_M_invokeERKSt9_Any_dataS7_ @ 0x7efebbfb2aea std::function<>::operator()() @ 0x7efebcb158b8 _ZZZN7process11ProcessBase6_visitERKNS0_12HttpEndpointERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEERKNS_5OwnedINS_4http7RequestNKUlRK6OptionINSD_14authentication20AuthenticationResultEEE0_clESN_ENKUlbE1_clEb @ 0x7efebcb1a10a _ZZZNK7process9_DeferredIZZNS_11ProcessBase6_visitERKNS1_12HttpEndpointERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEERKNS_5OwnedINS_4http7RequestNKUlRK6OptionINSE_14authentication20AuthenticationResultEEE0_clESO_EUlbE1_EcvSt8functionIFT_T0_EEINS_6FutureINSE_8ResponseEEERKbEEvENKUlS12_E_clES12_ENKUlvE_clEv @ 0x7efebcb1c5f8 _ZNSt17_Function_handlerIFN7process6FutureINS0_4http8ResponseEEEvEZZNKS0_9_DeferredIZZNS0_11ProcessBase6_visitERKNS7_12HttpEndpointERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEERKNS0_5OwnedINS2_7RequestNKUlRK6OptionINS2_14authentication20AuthenticationResultEEE0_clEST_EUlbE1_EcvSt8functionIFT_T0_EEIS4_RKbEEvENKUlS14_E_clES14_EUlvE_E9_M_invokeERKSt9_Any_data @ 0x7efebb5ce8ca std::function<>::operator()() @ 0x7efebb5c4b27 _ZZN7process8internal8DispatchINS_6FutureINS_4http8ResponseclIRSt8functionIFS5_vS5_RKNS_4UPIDEOT_ENKUlPNS_11ProcessBaseEE_clESI_ @ 0x7efebb5d4e1e _ZNSt17_Function_handlerIFvPN7process11ProcessBaseEEZNS0_8internal8DispatchINS0_6FutureINS0_4http8ResponseclIRSt8functionIFS9_vS9_RKNS0_4UPIDEOT_EUlS2_E_E9_M_invokeERKSt9_Any_dataOS2_ @ 0x7efebcb30baf std::function<>::operator()() @ 0x7efebcb13fd6 process::ProcessBase::visit() @ 0x7efebcb1f3c8 process::DispatchEvent::visit() @ 0x7efebb3ab2ea process::ProcessBase::serve() @ 0x7efebcb0fe8a process::ProcessManager::resume() @ 0x7efebcb0c5a3 _ZZN7process14ProcessManager12init_threadsEvENKUt_clEv @ 0x7efebcb1ea34 _ZNSt12_Bind_simpleIFZN7process14ProcessManager12init_threadsEvEUt_vEE9_M_invokeIJEEEvSt12_Index_tupleIJXspT_EEE @ 0x7efebcb1e98a _ZNSt12_Bind_simpleIFZN7process14ProcessManager12init_threadsEvEUt_vEEclEv @ 0x7efebcb1e91a _ZNSt6thread5_ImplISt12_Bind_simpleIFZN7process14ProcessManager12init_threadsEvEUt_vEEE6_M_runEv @ 0x7efeb6980c80 (unknown) @ 0x7efeb649c6ba start_thread @ 0x7efeb61d282d (unknown) Aborted (core dumped) ``` Diffs (updated) - src/slave/validation.cpp abd9b1248 src/tests/executor_http_api_tests.cpp 872caf6dd Diff: https://reviews.apache.org/r/55480/diff/ Testing --- Verified by making sure no segfault occurs when rebuilding Mesos with this fix and pointing our framework at it. Our framework is currently not generating v4 UUIDs which was exposing the issue. Thanks, Aaron Wood
Re: Review Request 55480: Fix segfault when the executor sets a UUID that is not a valid v4 UUID.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55480/ --- (Updated Jan. 13, 2017, 4:46 p.m.) Review request for mesos and Anand Mazumdar. Changes --- Adjusted one of the tests to set a bad UUID value. Bugs: MESOS-6917 https://issues.apache.org/jira/browse/MESOS-6917 Repository: mesos Description --- This fixes the segfault that occurs when an executor sets a UUID that's not a valid v4 UUID and sends it off to the agent: ``` ABORT: (../../3rdparty/stout/include/stout/try.hpp:77): Try::get() but state == ERROR: Not a valid UUID *** Aborted at 1484262968 (unix time) try "date -d @1484262968" if you are using GNU date *** PC: @ 0x7efeb6101428 (unknown) *** SIGABRT (@0x36b7) received by PID 14007 (TID 0x7efeabd29700) from PID 14007; stack trace: *** @ 0x7efeb64a6390 (unknown) @ 0x7efeb6101428 (unknown) @ 0x7efeb610302a (unknown) @ 0x560df739fa6e _Abort() @ 0x560df739fa9c _Abort() @ 0x7efebb53a5ad Try<>::get() @ 0x7efebb5363d6 Try<>::get() @ 0x7efebbd84809 mesos::internal::slave::validation::executor::call::validate() @ 0x7efebbb59b36 mesos::internal::slave::Slave::Http::executor() @ 0x7efebbc773b8 _ZZN5mesos8internal5slave5Slave10initializeEvENKUlRKN7process4http7RequestEE1_clES7_ @ 0x7efebbcb5808 _ZNSt17_Function_handlerIFN7process6FutureINS0_4http8ResponseEEERKNS2_7RequestEEZN5mesos8internal5slave5Slave10initializeEvEUlS7_E1_E9_M_invokeERKSt9_Any_dataS7_ @ 0x7efebbfb2aea std::function<>::operator()() @ 0x7efebcb158b8 _ZZZN7process11ProcessBase6_visitERKNS0_12HttpEndpointERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEERKNS_5OwnedINS_4http7RequestNKUlRK6OptionINSD_14authentication20AuthenticationResultEEE0_clESN_ENKUlbE1_clEb @ 0x7efebcb1a10a _ZZZNK7process9_DeferredIZZNS_11ProcessBase6_visitERKNS1_12HttpEndpointERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEERKNS_5OwnedINS_4http7RequestNKUlRK6OptionINSE_14authentication20AuthenticationResultEEE0_clESO_EUlbE1_EcvSt8functionIFT_T0_EEINS_6FutureINSE_8ResponseEEERKbEEvENKUlS12_E_clES12_ENKUlvE_clEv @ 0x7efebcb1c5f8 _ZNSt17_Function_handlerIFN7process6FutureINS0_4http8ResponseEEEvEZZNKS0_9_DeferredIZZNS0_11ProcessBase6_visitERKNS7_12HttpEndpointERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEERKNS0_5OwnedINS2_7RequestNKUlRK6OptionINS2_14authentication20AuthenticationResultEEE0_clEST_EUlbE1_EcvSt8functionIFT_T0_EEIS4_RKbEEvENKUlS14_E_clES14_EUlvE_E9_M_invokeERKSt9_Any_data @ 0x7efebb5ce8ca std::function<>::operator()() @ 0x7efebb5c4b27 _ZZN7process8internal8DispatchINS_6FutureINS_4http8ResponseclIRSt8functionIFS5_vS5_RKNS_4UPIDEOT_ENKUlPNS_11ProcessBaseEE_clESI_ @ 0x7efebb5d4e1e _ZNSt17_Function_handlerIFvPN7process11ProcessBaseEEZNS0_8internal8DispatchINS0_6FutureINS0_4http8ResponseclIRSt8functionIFS9_vS9_RKNS0_4UPIDEOT_EUlS2_E_E9_M_invokeERKSt9_Any_dataOS2_ @ 0x7efebcb30baf std::function<>::operator()() @ 0x7efebcb13fd6 process::ProcessBase::visit() @ 0x7efebcb1f3c8 process::DispatchEvent::visit() @ 0x7efebb3ab2ea process::ProcessBase::serve() @ 0x7efebcb0fe8a process::ProcessManager::resume() @ 0x7efebcb0c5a3 _ZZN7process14ProcessManager12init_threadsEvENKUt_clEv @ 0x7efebcb1ea34 _ZNSt12_Bind_simpleIFZN7process14ProcessManager12init_threadsEvEUt_vEE9_M_invokeIJEEEvSt12_Index_tupleIJXspT_EEE @ 0x7efebcb1e98a _ZNSt12_Bind_simpleIFZN7process14ProcessManager12init_threadsEvEUt_vEEclEv @ 0x7efebcb1e91a _ZNSt6thread5_ImplISt12_Bind_simpleIFZN7process14ProcessManager12init_threadsEvEUt_vEEE6_M_runEv @ 0x7efeb6980c80 (unknown) @ 0x7efeb649c6ba start_thread @ 0x7efeb61d282d (unknown) Aborted (core dumped) ``` Diffs (updated) - src/slave/validation.cpp abd9b12 src/tests/executor_http_api_tests.cpp 872caf6 Diff: https://reviews.apache.org/r/55480/diff/ Testing --- Verified by making sure no segfault occurs when rebuilding Mesos with this fix and pointing our framework at it. Our framework is currently not generating v4 UUIDs which was exposing the issue. Thanks, Aaron Wood
Re: Review Request 55480: Fix segfault when the executor sets a UUID that is not a valid v4 UUID.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55480/ --- (Updated Jan. 13, 2017, 3:56 p.m.) Review request for mesos and Anand Mazumdar. Bugs: MESOS-6917 https://issues.apache.org/jira/browse/MESOS-6917 Repository: mesos Description --- This fixes the segfault that occurs when an executor sets a UUID that's not a valid v4 UUID and sends it off to the agent: ``` ABORT: (../../3rdparty/stout/include/stout/try.hpp:77): Try::get() but state == ERROR: Not a valid UUID *** Aborted at 1484262968 (unix time) try "date -d @1484262968" if you are using GNU date *** PC: @ 0x7efeb6101428 (unknown) *** SIGABRT (@0x36b7) received by PID 14007 (TID 0x7efeabd29700) from PID 14007; stack trace: *** @ 0x7efeb64a6390 (unknown) @ 0x7efeb6101428 (unknown) @ 0x7efeb610302a (unknown) @ 0x560df739fa6e _Abort() @ 0x560df739fa9c _Abort() @ 0x7efebb53a5ad Try<>::get() @ 0x7efebb5363d6 Try<>::get() @ 0x7efebbd84809 mesos::internal::slave::validation::executor::call::validate() @ 0x7efebbb59b36 mesos::internal::slave::Slave::Http::executor() @ 0x7efebbc773b8 _ZZN5mesos8internal5slave5Slave10initializeEvENKUlRKN7process4http7RequestEE1_clES7_ @ 0x7efebbcb5808 _ZNSt17_Function_handlerIFN7process6FutureINS0_4http8ResponseEEERKNS2_7RequestEEZN5mesos8internal5slave5Slave10initializeEvEUlS7_E1_E9_M_invokeERKSt9_Any_dataS7_ @ 0x7efebbfb2aea std::function<>::operator()() @ 0x7efebcb158b8 _ZZZN7process11ProcessBase6_visitERKNS0_12HttpEndpointERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEERKNS_5OwnedINS_4http7RequestNKUlRK6OptionINSD_14authentication20AuthenticationResultEEE0_clESN_ENKUlbE1_clEb @ 0x7efebcb1a10a _ZZZNK7process9_DeferredIZZNS_11ProcessBase6_visitERKNS1_12HttpEndpointERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEERKNS_5OwnedINS_4http7RequestNKUlRK6OptionINSE_14authentication20AuthenticationResultEEE0_clESO_EUlbE1_EcvSt8functionIFT_T0_EEINS_6FutureINSE_8ResponseEEERKbEEvENKUlS12_E_clES12_ENKUlvE_clEv @ 0x7efebcb1c5f8 _ZNSt17_Function_handlerIFN7process6FutureINS0_4http8ResponseEEEvEZZNKS0_9_DeferredIZZNS0_11ProcessBase6_visitERKNS7_12HttpEndpointERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEERKNS0_5OwnedINS2_7RequestNKUlRK6OptionINS2_14authentication20AuthenticationResultEEE0_clEST_EUlbE1_EcvSt8functionIFT_T0_EEIS4_RKbEEvENKUlS14_E_clES14_EUlvE_E9_M_invokeERKSt9_Any_data @ 0x7efebb5ce8ca std::function<>::operator()() @ 0x7efebb5c4b27 _ZZN7process8internal8DispatchINS_6FutureINS_4http8ResponseclIRSt8functionIFS5_vS5_RKNS_4UPIDEOT_ENKUlPNS_11ProcessBaseEE_clESI_ @ 0x7efebb5d4e1e _ZNSt17_Function_handlerIFvPN7process11ProcessBaseEEZNS0_8internal8DispatchINS0_6FutureINS0_4http8ResponseclIRSt8functionIFS9_vS9_RKNS0_4UPIDEOT_EUlS2_E_E9_M_invokeERKSt9_Any_dataOS2_ @ 0x7efebcb30baf std::function<>::operator()() @ 0x7efebcb13fd6 process::ProcessBase::visit() @ 0x7efebcb1f3c8 process::DispatchEvent::visit() @ 0x7efebb3ab2ea process::ProcessBase::serve() @ 0x7efebcb0fe8a process::ProcessManager::resume() @ 0x7efebcb0c5a3 _ZZN7process14ProcessManager12init_threadsEvENKUt_clEv @ 0x7efebcb1ea34 _ZNSt12_Bind_simpleIFZN7process14ProcessManager12init_threadsEvEUt_vEE9_M_invokeIJEEEvSt12_Index_tupleIJXspT_EEE @ 0x7efebcb1e98a _ZNSt12_Bind_simpleIFZN7process14ProcessManager12init_threadsEvEUt_vEEclEv @ 0x7efebcb1e91a _ZNSt6thread5_ImplISt12_Bind_simpleIFZN7process14ProcessManager12init_threadsEvEUt_vEEE6_M_runEv @ 0x7efeb6980c80 (unknown) @ 0x7efeb649c6ba start_thread @ 0x7efeb61d282d (unknown) Aborted (core dumped) ``` Diffs - src/slave/validation.cpp abd9b1248 Diff: https://reviews.apache.org/r/55480/diff/ Testing --- Verified by making sure no segfault occurs when rebuilding Mesos with this fix and pointing our framework at it. Our framework is currently not generating v4 UUIDs which was exposing the issue. Thanks, Aaron Wood
Re: Review Request 55480: Fix segfault when the executor sets a UUID that is not a valid v4 UUID.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55480/ --- (Updated Jan. 13, 2017, 3:52 p.m.) Review request for mesos and Anand Mazumdar. Summary (updated) - Fix segfault when the executor sets a UUID that is not a valid v4 UUID. Repository: mesos Description --- This fixes the segfault that occurs when an executor sets a UUID that's not a valid v4 UUID and sends it off to the agent: ``` ABORT: (../../3rdparty/stout/include/stout/try.hpp:77): Try::get() but state == ERROR: Not a valid UUID *** Aborted at 1484262968 (unix time) try "date -d @1484262968" if you are using GNU date *** PC: @ 0x7efeb6101428 (unknown) *** SIGABRT (@0x36b7) received by PID 14007 (TID 0x7efeabd29700) from PID 14007; stack trace: *** @ 0x7efeb64a6390 (unknown) @ 0x7efeb6101428 (unknown) @ 0x7efeb610302a (unknown) @ 0x560df739fa6e _Abort() @ 0x560df739fa9c _Abort() @ 0x7efebb53a5ad Try<>::get() @ 0x7efebb5363d6 Try<>::get() @ 0x7efebbd84809 mesos::internal::slave::validation::executor::call::validate() @ 0x7efebbb59b36 mesos::internal::slave::Slave::Http::executor() @ 0x7efebbc773b8 _ZZN5mesos8internal5slave5Slave10initializeEvENKUlRKN7process4http7RequestEE1_clES7_ @ 0x7efebbcb5808 _ZNSt17_Function_handlerIFN7process6FutureINS0_4http8ResponseEEERKNS2_7RequestEEZN5mesos8internal5slave5Slave10initializeEvEUlS7_E1_E9_M_invokeERKSt9_Any_dataS7_ @ 0x7efebbfb2aea std::function<>::operator()() @ 0x7efebcb158b8 _ZZZN7process11ProcessBase6_visitERKNS0_12HttpEndpointERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEERKNS_5OwnedINS_4http7RequestNKUlRK6OptionINSD_14authentication20AuthenticationResultEEE0_clESN_ENKUlbE1_clEb @ 0x7efebcb1a10a _ZZZNK7process9_DeferredIZZNS_11ProcessBase6_visitERKNS1_12HttpEndpointERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEERKNS_5OwnedINS_4http7RequestNKUlRK6OptionINSE_14authentication20AuthenticationResultEEE0_clESO_EUlbE1_EcvSt8functionIFT_T0_EEINS_6FutureINSE_8ResponseEEERKbEEvENKUlS12_E_clES12_ENKUlvE_clEv @ 0x7efebcb1c5f8 _ZNSt17_Function_handlerIFN7process6FutureINS0_4http8ResponseEEEvEZZNKS0_9_DeferredIZZNS0_11ProcessBase6_visitERKNS7_12HttpEndpointERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEERKNS0_5OwnedINS2_7RequestNKUlRK6OptionINS2_14authentication20AuthenticationResultEEE0_clEST_EUlbE1_EcvSt8functionIFT_T0_EEIS4_RKbEEvENKUlS14_E_clES14_EUlvE_E9_M_invokeERKSt9_Any_data @ 0x7efebb5ce8ca std::function<>::operator()() @ 0x7efebb5c4b27 _ZZN7process8internal8DispatchINS_6FutureINS_4http8ResponseclIRSt8functionIFS5_vS5_RKNS_4UPIDEOT_ENKUlPNS_11ProcessBaseEE_clESI_ @ 0x7efebb5d4e1e _ZNSt17_Function_handlerIFvPN7process11ProcessBaseEEZNS0_8internal8DispatchINS0_6FutureINS0_4http8ResponseclIRSt8functionIFS9_vS9_RKNS0_4UPIDEOT_EUlS2_E_E9_M_invokeERKSt9_Any_dataOS2_ @ 0x7efebcb30baf std::function<>::operator()() @ 0x7efebcb13fd6 process::ProcessBase::visit() @ 0x7efebcb1f3c8 process::DispatchEvent::visit() @ 0x7efebb3ab2ea process::ProcessBase::serve() @ 0x7efebcb0fe8a process::ProcessManager::resume() @ 0x7efebcb0c5a3 _ZZN7process14ProcessManager12init_threadsEvENKUt_clEv @ 0x7efebcb1ea34 _ZNSt12_Bind_simpleIFZN7process14ProcessManager12init_threadsEvEUt_vEE9_M_invokeIJEEEvSt12_Index_tupleIJXspT_EEE @ 0x7efebcb1e98a _ZNSt12_Bind_simpleIFZN7process14ProcessManager12init_threadsEvEUt_vEEclEv @ 0x7efebcb1e91a _ZNSt6thread5_ImplISt12_Bind_simpleIFZN7process14ProcessManager12init_threadsEvEUt_vEEE6_M_runEv @ 0x7efeb6980c80 (unknown) @ 0x7efeb649c6ba start_thread @ 0x7efeb61d282d (unknown) Aborted (core dumped) ``` Diffs - src/slave/validation.cpp abd9b1248 Diff: https://reviews.apache.org/r/55480/diff/ Testing --- Verified by making sure no segfault occurs when rebuilding Mesos with this fix and pointing our framework at it. Our framework is currently not generating v4 UUIDs which was exposing the issue. Thanks, Aaron Wood
Re: Review Request 55480: Fix segfault when the executor ID is not a valid v4 UUID.
> On Jan. 13, 2017, 4:52 a.m., Vinod Kone wrote: > > Thanks for the fix! > > > > Would you mind updating the `StatusUpdateCallFailedValidation` test in > > executor_http_api_tests.cpp to include this case? > > Vinod Kone wrote: > Also if you can create a ticket for this, and set target versions to 1.0, > 1.1 and 1.2 with "blocker" priority, we will make sure to backport it. Thank you! Unfortunately JIRA looks like it's still down. I'll be sure to create that ticket ASAP. - Aaron --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55480/#review161484 ------- On Jan. 12, 2017, 11:52 p.m., Aaron Wood wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/55480/ > --- > > (Updated Jan. 12, 2017, 11:52 p.m.) > > > Review request for mesos and Anand Mazumdar. > > > Repository: mesos > > > Description > --- > > This fixes the segfault that occurs when an executor sets a UUID that's not a > valid v4 UUID and sends it off to the agent: > > ``` > ABORT: (../../3rdparty/stout/include/stout/try.hpp:77): Try::get() but state > == ERROR: Not a valid UUID > *** Aborted at 1484262968 (unix time) try "date -d @1484262968" if you are > using GNU date *** > PC: @ 0x7efeb6101428 (unknown) > *** SIGABRT (@0x36b7) received by PID 14007 (TID 0x7efeabd29700) from PID > 14007; stack trace: *** > @ 0x7efeb64a6390 (unknown) > @ 0x7efeb6101428 (unknown) > @ 0x7efeb610302a (unknown) > @ 0x560df739fa6e _Abort() > @ 0x560df739fa9c _Abort() > @ 0x7efebb53a5ad Try<>::get() > @ 0x7efebb5363d6 Try<>::get() > @ 0x7efebbd84809 > mesos::internal::slave::validation::executor::call::validate() > @ 0x7efebbb59b36 mesos::internal::slave::Slave::Http::executor() > @ 0x7efebbc773b8 > _ZZN5mesos8internal5slave5Slave10initializeEvENKUlRKN7process4http7RequestEE1_clES7_ > @ 0x7efebbcb5808 > _ZNSt17_Function_handlerIFN7process6FutureINS0_4http8ResponseEEERKNS2_7RequestEEZN5mesos8internal5slave5Slave10initializeEvEUlS7_E1_E9_M_invokeERKSt9_Any_dataS7_ > @ 0x7efebbfb2aea std::function<>::operator()() > @ 0x7efebcb158b8 > _ZZZN7process11ProcessBase6_visitERKNS0_12HttpEndpointERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEERKNS_5OwnedINS_4http7RequestNKUlRK6OptionINSD_14authentication20AuthenticationResultEEE0_clESN_ENKUlbE1_clEb > @ 0x7efebcb1a10a > _ZZZNK7process9_DeferredIZZNS_11ProcessBase6_visitERKNS1_12HttpEndpointERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEERKNS_5OwnedINS_4http7RequestNKUlRK6OptionINSE_14authentication20AuthenticationResultEEE0_clESO_EUlbE1_EcvSt8functionIFT_T0_EEINS_6FutureINSE_8ResponseEEERKbEEvENKUlS12_E_clES12_ENKUlvE_clEv > @ 0x7efebcb1c5f8 > _ZNSt17_Function_handlerIFN7process6FutureINS0_4http8ResponseEEEvEZZNKS0_9_DeferredIZZNS0_11ProcessBase6_visitERKNS7_12HttpEndpointERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEERKNS0_5OwnedINS2_7RequestNKUlRK6OptionINS2_14authentication20AuthenticationResultEEE0_clEST_EUlbE1_EcvSt8functionIFT_T0_EEIS4_RKbEEvENKUlS14_E_clES14_EUlvE_E9_M_invokeERKSt9_Any_data > @ 0x7efebb5ce8ca std::function<>::operator()() > @ 0x7efebb5c4b27 > _ZZN7process8internal8DispatchINS_6FutureINS_4http8ResponseclIRSt8functionIFS5_vS5_RKNS_4UPIDEOT_ENKUlPNS_11ProcessBaseEE_clESI_ > @ 0x7efebb5d4e1e > _ZNSt17_Function_handlerIFvPN7process11ProcessBaseEEZNS0_8internal8DispatchINS0_6FutureINS0_4http8ResponseclIRSt8functionIFS9_vS9_RKNS0_4UPIDEOT_EUlS2_E_E9_M_invokeERKSt9_Any_dataOS2_ > @ 0x7efebcb30baf std::function<>::operator()() > @ 0x7efebcb13fd6 process::ProcessBase::visit() > @ 0x7efebcb1f3c8 process::DispatchEvent::visit() > @ 0x7efebb3ab2ea process::ProcessBase::serve() > @ 0x7efebcb0fe8a process::ProcessManager::resume() > @ 0x7efebcb0c5a3 > _ZZN7process14ProcessManager12init_threadsEvENKUt_clEv > @ 0x7efebcb1ea34 > _ZNSt12_Bind_simpleIFZN7process14ProcessManager12init_threadsEvEUt_vEE9_M_invokeIJEEEvSt12_Index_tupleIJXspT_EEE > @ 0x7efebcb1e98a > _ZNSt12_Bind_simpleIFZN7process14ProcessManager12init_threadsEvEUt_vEEclEv > @ 0x7efebcb1e91a > _ZNSt6thread5_ImplISt12_Bind_simpleIFZN7process14ProcessManager12init_threadsEvEUt_vEEE6_M_runEv > @ 0x7efeb6980c80 (unknown) > @ 0x7efeb6
Re: Review Request 55480: Fix segfault when the executor ID is not a valid v4 UUID.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55480/ --- (Updated Jan. 12, 2017, 11:52 p.m.) Review request for mesos and Anand Mazumdar. Repository: mesos Description --- This fixes the segfault that occurs when an executor sets a UUID that's not a valid v4 UUID and sends it off to the agent: ``` ABORT: (../../3rdparty/stout/include/stout/try.hpp:77): Try::get() but state == ERROR: Not a valid UUID *** Aborted at 1484262968 (unix time) try "date -d @1484262968" if you are using GNU date *** PC: @ 0x7efeb6101428 (unknown) *** SIGABRT (@0x36b7) received by PID 14007 (TID 0x7efeabd29700) from PID 14007; stack trace: *** @ 0x7efeb64a6390 (unknown) @ 0x7efeb6101428 (unknown) @ 0x7efeb610302a (unknown) @ 0x560df739fa6e _Abort() @ 0x560df739fa9c _Abort() @ 0x7efebb53a5ad Try<>::get() @ 0x7efebb5363d6 Try<>::get() @ 0x7efebbd84809 mesos::internal::slave::validation::executor::call::validate() @ 0x7efebbb59b36 mesos::internal::slave::Slave::Http::executor() @ 0x7efebbc773b8 _ZZN5mesos8internal5slave5Slave10initializeEvENKUlRKN7process4http7RequestEE1_clES7_ @ 0x7efebbcb5808 _ZNSt17_Function_handlerIFN7process6FutureINS0_4http8ResponseEEERKNS2_7RequestEEZN5mesos8internal5slave5Slave10initializeEvEUlS7_E1_E9_M_invokeERKSt9_Any_dataS7_ @ 0x7efebbfb2aea std::function<>::operator()() @ 0x7efebcb158b8 _ZZZN7process11ProcessBase6_visitERKNS0_12HttpEndpointERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEERKNS_5OwnedINS_4http7RequestNKUlRK6OptionINSD_14authentication20AuthenticationResultEEE0_clESN_ENKUlbE1_clEb @ 0x7efebcb1a10a _ZZZNK7process9_DeferredIZZNS_11ProcessBase6_visitERKNS1_12HttpEndpointERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEERKNS_5OwnedINS_4http7RequestNKUlRK6OptionINSE_14authentication20AuthenticationResultEEE0_clESO_EUlbE1_EcvSt8functionIFT_T0_EEINS_6FutureINSE_8ResponseEEERKbEEvENKUlS12_E_clES12_ENKUlvE_clEv @ 0x7efebcb1c5f8 _ZNSt17_Function_handlerIFN7process6FutureINS0_4http8ResponseEEEvEZZNKS0_9_DeferredIZZNS0_11ProcessBase6_visitERKNS7_12HttpEndpointERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEERKNS0_5OwnedINS2_7RequestNKUlRK6OptionINS2_14authentication20AuthenticationResultEEE0_clEST_EUlbE1_EcvSt8functionIFT_T0_EEIS4_RKbEEvENKUlS14_E_clES14_EUlvE_E9_M_invokeERKSt9_Any_data @ 0x7efebb5ce8ca std::function<>::operator()() @ 0x7efebb5c4b27 _ZZN7process8internal8DispatchINS_6FutureINS_4http8ResponseclIRSt8functionIFS5_vS5_RKNS_4UPIDEOT_ENKUlPNS_11ProcessBaseEE_clESI_ @ 0x7efebb5d4e1e _ZNSt17_Function_handlerIFvPN7process11ProcessBaseEEZNS0_8internal8DispatchINS0_6FutureINS0_4http8ResponseclIRSt8functionIFS9_vS9_RKNS0_4UPIDEOT_EUlS2_E_E9_M_invokeERKSt9_Any_dataOS2_ @ 0x7efebcb30baf std::function<>::operator()() @ 0x7efebcb13fd6 process::ProcessBase::visit() @ 0x7efebcb1f3c8 process::DispatchEvent::visit() @ 0x7efebb3ab2ea process::ProcessBase::serve() @ 0x7efebcb0fe8a process::ProcessManager::resume() @ 0x7efebcb0c5a3 _ZZN7process14ProcessManager12init_threadsEvENKUt_clEv @ 0x7efebcb1ea34 _ZNSt12_Bind_simpleIFZN7process14ProcessManager12init_threadsEvEUt_vEE9_M_invokeIJEEEvSt12_Index_tupleIJXspT_EEE @ 0x7efebcb1e98a _ZNSt12_Bind_simpleIFZN7process14ProcessManager12init_threadsEvEUt_vEEclEv @ 0x7efebcb1e91a _ZNSt6thread5_ImplISt12_Bind_simpleIFZN7process14ProcessManager12init_threadsEvEUt_vEEE6_M_runEv @ 0x7efeb6980c80 (unknown) @ 0x7efeb649c6ba start_thread @ 0x7efeb61d282d (unknown) Aborted (core dumped) ``` Diffs - src/slave/validation.cpp abd9b1248 Diff: https://reviews.apache.org/r/55480/diff/ Testing (updated) --- Verified by making sure no segfault occurs when rebuilding Mesos with this fix and pointing our framework at it. Our framework is currently not generating v4 UUIDs which was exposing the issue. Thanks, Aaron Wood
Re: Review Request 55480: Fix segfault when the executor ID is not a valid v4 UUID.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55480/ --- (Updated Jan. 12, 2017, 11:41 p.m.) Review request for mesos and Anand Mazumdar. Summary (updated) - Fix segfault when the executor ID is not a valid v4 UUID. Repository: mesos Description --- This fixes the segfault that occurs when an executor sets a UUID that's not a valid v4 UUID and sends it off to the agent: ``` ABORT: (../../3rdparty/stout/include/stout/try.hpp:77): Try::get() but state == ERROR: Not a valid UUID *** Aborted at 1484262968 (unix time) try "date -d @1484262968" if you are using GNU date *** PC: @ 0x7efeb6101428 (unknown) *** SIGABRT (@0x36b7) received by PID 14007 (TID 0x7efeabd29700) from PID 14007; stack trace: *** @ 0x7efeb64a6390 (unknown) @ 0x7efeb6101428 (unknown) @ 0x7efeb610302a (unknown) @ 0x560df739fa6e _Abort() @ 0x560df739fa9c _Abort() @ 0x7efebb53a5ad Try<>::get() @ 0x7efebb5363d6 Try<>::get() @ 0x7efebbd84809 mesos::internal::slave::validation::executor::call::validate() @ 0x7efebbb59b36 mesos::internal::slave::Slave::Http::executor() @ 0x7efebbc773b8 _ZZN5mesos8internal5slave5Slave10initializeEvENKUlRKN7process4http7RequestEE1_clES7_ @ 0x7efebbcb5808 _ZNSt17_Function_handlerIFN7process6FutureINS0_4http8ResponseEEERKNS2_7RequestEEZN5mesos8internal5slave5Slave10initializeEvEUlS7_E1_E9_M_invokeERKSt9_Any_dataS7_ @ 0x7efebbfb2aea std::function<>::operator()() @ 0x7efebcb158b8 _ZZZN7process11ProcessBase6_visitERKNS0_12HttpEndpointERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEERKNS_5OwnedINS_4http7RequestNKUlRK6OptionINSD_14authentication20AuthenticationResultEEE0_clESN_ENKUlbE1_clEb @ 0x7efebcb1a10a _ZZZNK7process9_DeferredIZZNS_11ProcessBase6_visitERKNS1_12HttpEndpointERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEERKNS_5OwnedINS_4http7RequestNKUlRK6OptionINSE_14authentication20AuthenticationResultEEE0_clESO_EUlbE1_EcvSt8functionIFT_T0_EEINS_6FutureINSE_8ResponseEEERKbEEvENKUlS12_E_clES12_ENKUlvE_clEv @ 0x7efebcb1c5f8 _ZNSt17_Function_handlerIFN7process6FutureINS0_4http8ResponseEEEvEZZNKS0_9_DeferredIZZNS0_11ProcessBase6_visitERKNS7_12HttpEndpointERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEERKNS0_5OwnedINS2_7RequestNKUlRK6OptionINS2_14authentication20AuthenticationResultEEE0_clEST_EUlbE1_EcvSt8functionIFT_T0_EEIS4_RKbEEvENKUlS14_E_clES14_EUlvE_E9_M_invokeERKSt9_Any_data @ 0x7efebb5ce8ca std::function<>::operator()() @ 0x7efebb5c4b27 _ZZN7process8internal8DispatchINS_6FutureINS_4http8ResponseclIRSt8functionIFS5_vS5_RKNS_4UPIDEOT_ENKUlPNS_11ProcessBaseEE_clESI_ @ 0x7efebb5d4e1e _ZNSt17_Function_handlerIFvPN7process11ProcessBaseEEZNS0_8internal8DispatchINS0_6FutureINS0_4http8ResponseclIRSt8functionIFS9_vS9_RKNS0_4UPIDEOT_EUlS2_E_E9_M_invokeERKSt9_Any_dataOS2_ @ 0x7efebcb30baf std::function<>::operator()() @ 0x7efebcb13fd6 process::ProcessBase::visit() @ 0x7efebcb1f3c8 process::DispatchEvent::visit() @ 0x7efebb3ab2ea process::ProcessBase::serve() @ 0x7efebcb0fe8a process::ProcessManager::resume() @ 0x7efebcb0c5a3 _ZZN7process14ProcessManager12init_threadsEvENKUt_clEv @ 0x7efebcb1ea34 _ZNSt12_Bind_simpleIFZN7process14ProcessManager12init_threadsEvEUt_vEE9_M_invokeIJEEEvSt12_Index_tupleIJXspT_EEE @ 0x7efebcb1e98a _ZNSt12_Bind_simpleIFZN7process14ProcessManager12init_threadsEvEUt_vEEclEv @ 0x7efebcb1e91a _ZNSt6thread5_ImplISt12_Bind_simpleIFZN7process14ProcessManager12init_threadsEvEUt_vEEE6_M_runEv @ 0x7efeb6980c80 (unknown) @ 0x7efeb649c6ba start_thread @ 0x7efeb61d282d (unknown) Aborted (core dumped) ``` Diffs - src/slave/validation.cpp abd9b1248 Diff: https://reviews.apache.org/r/55480/diff/ Testing --- Verified by making sure no segfault occurs when rebuilding Mesos with this fix and pointing our framework at it. Our framework is currently not generating v4 UUIDs which was causing the issue. Thanks, Aaron Wood
Re: Review Request 55480: Fix segfault when executor ID is not a valid UUID.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55480/#review161455 --- Will create a JIRA for this as soon as it's back online. - Aaron Wood On Jan. 12, 2017, 11:23 p.m., Aaron Wood wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/55480/ > --- > > (Updated Jan. 12, 2017, 11:23 p.m.) > > > Review request for mesos and Anand Mazumdar. > > > Repository: mesos > > > Description > --- > > This fixes the segfault that occurs when an executor sets a UUID that's not a > valid v4 UUID and sends it off to the agent: > > ``` > ABORT: (../../3rdparty/stout/include/stout/try.hpp:77): Try::get() but state > == ERROR: Not a valid UUID > *** Aborted at 1484262968 (unix time) try "date -d @1484262968" if you are > using GNU date *** > PC: @ 0x7efeb6101428 (unknown) > *** SIGABRT (@0x36b7) received by PID 14007 (TID 0x7efeabd29700) from PID > 14007; stack trace: *** > @ 0x7efeb64a6390 (unknown) > @ 0x7efeb6101428 (unknown) > @ 0x7efeb610302a (unknown) > @ 0x560df739fa6e _Abort() > @ 0x560df739fa9c _Abort() > @ 0x7efebb53a5ad Try<>::get() > @ 0x7efebb5363d6 Try<>::get() > @ 0x7efebbd84809 > mesos::internal::slave::validation::executor::call::validate() > @ 0x7efebbb59b36 mesos::internal::slave::Slave::Http::executor() > @ 0x7efebbc773b8 > _ZZN5mesos8internal5slave5Slave10initializeEvENKUlRKN7process4http7RequestEE1_clES7_ > @ 0x7efebbcb5808 > _ZNSt17_Function_handlerIFN7process6FutureINS0_4http8ResponseEEERKNS2_7RequestEEZN5mesos8internal5slave5Slave10initializeEvEUlS7_E1_E9_M_invokeERKSt9_Any_dataS7_ > @ 0x7efebbfb2aea std::function<>::operator()() > @ 0x7efebcb158b8 > _ZZZN7process11ProcessBase6_visitERKNS0_12HttpEndpointERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEERKNS_5OwnedINS_4http7RequestNKUlRK6OptionINSD_14authentication20AuthenticationResultEEE0_clESN_ENKUlbE1_clEb > @ 0x7efebcb1a10a > _ZZZNK7process9_DeferredIZZNS_11ProcessBase6_visitERKNS1_12HttpEndpointERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEERKNS_5OwnedINS_4http7RequestNKUlRK6OptionINSE_14authentication20AuthenticationResultEEE0_clESO_EUlbE1_EcvSt8functionIFT_T0_EEINS_6FutureINSE_8ResponseEEERKbEEvENKUlS12_E_clES12_ENKUlvE_clEv > @ 0x7efebcb1c5f8 > _ZNSt17_Function_handlerIFN7process6FutureINS0_4http8ResponseEEEvEZZNKS0_9_DeferredIZZNS0_11ProcessBase6_visitERKNS7_12HttpEndpointERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEERKNS0_5OwnedINS2_7RequestNKUlRK6OptionINS2_14authentication20AuthenticationResultEEE0_clEST_EUlbE1_EcvSt8functionIFT_T0_EEIS4_RKbEEvENKUlS14_E_clES14_EUlvE_E9_M_invokeERKSt9_Any_data > @ 0x7efebb5ce8ca std::function<>::operator()() > @ 0x7efebb5c4b27 > _ZZN7process8internal8DispatchINS_6FutureINS_4http8ResponseclIRSt8functionIFS5_vS5_RKNS_4UPIDEOT_ENKUlPNS_11ProcessBaseEE_clESI_ > @ 0x7efebb5d4e1e > _ZNSt17_Function_handlerIFvPN7process11ProcessBaseEEZNS0_8internal8DispatchINS0_6FutureINS0_4http8ResponseclIRSt8functionIFS9_vS9_RKNS0_4UPIDEOT_EUlS2_E_E9_M_invokeERKSt9_Any_dataOS2_ > @ 0x7efebcb30baf std::function<>::operator()() > @ 0x7efebcb13fd6 process::ProcessBase::visit() > @ 0x7efebcb1f3c8 process::DispatchEvent::visit() > @ 0x7efebb3ab2ea process::ProcessBase::serve() > @ 0x7efebcb0fe8a process::ProcessManager::resume() > @ 0x7efebcb0c5a3 > _ZZN7process14ProcessManager12init_threadsEvENKUt_clEv > @ 0x7efebcb1ea34 > _ZNSt12_Bind_simpleIFZN7process14ProcessManager12init_threadsEvEUt_vEE9_M_invokeIJEEEvSt12_Index_tupleIJXspT_EEE > @ 0x7efebcb1e98a > _ZNSt12_Bind_simpleIFZN7process14ProcessManager12init_threadsEvEUt_vEEclEv > @ 0x7efebcb1e91a > _ZNSt6thread5_ImplISt12_Bind_simpleIFZN7process14ProcessManager12init_threadsEvEUt_vEEE6_M_runEv > @ 0x7efeb6980c80 (unknown) > @ 0x7efeb649c6ba start_thread > @ 0x7efeb61d282d (unknown) > Aborted (core dumped) > ``` > > > Diffs > - > > src/slave/validation.cpp abd9b1248 > > Diff: https://reviews.apache.org/r/55480/diff/ > > > Testing > --- > > Verified by making sure no segfault occurs when rebuilding Mesos with this > fix and pointing our framework at it. Our framework is currently not > generating v4 UUIDs which was causing the issue. > > > Thanks, > > Aaron Wood > >
Review Request 55480: Fix segfault when executor ID is not a valid UUID.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55480/ --- Review request for mesos and Anand Mazumdar. Repository: mesos Description --- This fixes the segfault that occurs when an executor sets a UUID that's not a valid v4 UUID and sends it off to the agent: ``` ABORT: (../../3rdparty/stout/include/stout/try.hpp:77): Try::get() but state == ERROR: Not a valid UUID *** Aborted at 1484262968 (unix time) try "date -d @1484262968" if you are using GNU date *** PC: @ 0x7efeb6101428 (unknown) *** SIGABRT (@0x36b7) received by PID 14007 (TID 0x7efeabd29700) from PID 14007; stack trace: *** @ 0x7efeb64a6390 (unknown) @ 0x7efeb6101428 (unknown) @ 0x7efeb610302a (unknown) @ 0x560df739fa6e _Abort() @ 0x560df739fa9c _Abort() @ 0x7efebb53a5ad Try<>::get() @ 0x7efebb5363d6 Try<>::get() @ 0x7efebbd84809 mesos::internal::slave::validation::executor::call::validate() @ 0x7efebbb59b36 mesos::internal::slave::Slave::Http::executor() @ 0x7efebbc773b8 _ZZN5mesos8internal5slave5Slave10initializeEvENKUlRKN7process4http7RequestEE1_clES7_ @ 0x7efebbcb5808 _ZNSt17_Function_handlerIFN7process6FutureINS0_4http8ResponseEEERKNS2_7RequestEEZN5mesos8internal5slave5Slave10initializeEvEUlS7_E1_E9_M_invokeERKSt9_Any_dataS7_ @ 0x7efebbfb2aea std::function<>::operator()() @ 0x7efebcb158b8 _ZZZN7process11ProcessBase6_visitERKNS0_12HttpEndpointERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEERKNS_5OwnedINS_4http7RequestNKUlRK6OptionINSD_14authentication20AuthenticationResultEEE0_clESN_ENKUlbE1_clEb @ 0x7efebcb1a10a _ZZZNK7process9_DeferredIZZNS_11ProcessBase6_visitERKNS1_12HttpEndpointERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEERKNS_5OwnedINS_4http7RequestNKUlRK6OptionINSE_14authentication20AuthenticationResultEEE0_clESO_EUlbE1_EcvSt8functionIFT_T0_EEINS_6FutureINSE_8ResponseEEERKbEEvENKUlS12_E_clES12_ENKUlvE_clEv @ 0x7efebcb1c5f8 _ZNSt17_Function_handlerIFN7process6FutureINS0_4http8ResponseEEEvEZZNKS0_9_DeferredIZZNS0_11ProcessBase6_visitERKNS7_12HttpEndpointERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEERKNS0_5OwnedINS2_7RequestNKUlRK6OptionINS2_14authentication20AuthenticationResultEEE0_clEST_EUlbE1_EcvSt8functionIFT_T0_EEIS4_RKbEEvENKUlS14_E_clES14_EUlvE_E9_M_invokeERKSt9_Any_data @ 0x7efebb5ce8ca std::function<>::operator()() @ 0x7efebb5c4b27 _ZZN7process8internal8DispatchINS_6FutureINS_4http8ResponseclIRSt8functionIFS5_vS5_RKNS_4UPIDEOT_ENKUlPNS_11ProcessBaseEE_clESI_ @ 0x7efebb5d4e1e _ZNSt17_Function_handlerIFvPN7process11ProcessBaseEEZNS0_8internal8DispatchINS0_6FutureINS0_4http8ResponseclIRSt8functionIFS9_vS9_RKNS0_4UPIDEOT_EUlS2_E_E9_M_invokeERKSt9_Any_dataOS2_ @ 0x7efebcb30baf std::function<>::operator()() @ 0x7efebcb13fd6 process::ProcessBase::visit() @ 0x7efebcb1f3c8 process::DispatchEvent::visit() @ 0x7efebb3ab2ea process::ProcessBase::serve() @ 0x7efebcb0fe8a process::ProcessManager::resume() @ 0x7efebcb0c5a3 _ZZN7process14ProcessManager12init_threadsEvENKUt_clEv @ 0x7efebcb1ea34 _ZNSt12_Bind_simpleIFZN7process14ProcessManager12init_threadsEvEUt_vEE9_M_invokeIJEEEvSt12_Index_tupleIJXspT_EEE @ 0x7efebcb1e98a _ZNSt12_Bind_simpleIFZN7process14ProcessManager12init_threadsEvEUt_vEEclEv @ 0x7efebcb1e91a _ZNSt6thread5_ImplISt12_Bind_simpleIFZN7process14ProcessManager12init_threadsEvEUt_vEEE6_M_runEv @ 0x7efeb6980c80 (unknown) @ 0x7efeb649c6ba start_thread @ 0x7efeb61d282d (unknown) Aborted (core dumped) ``` Diffs - src/slave/validation.cpp abd9b1248 Diff: https://reviews.apache.org/r/55480/diff/ Testing --- Verified by making sure no segfault occurs when rebuilding Mesos with this fix and pointing our framework at it. Our framework is currently not generating v4 UUIDs which was causing the issue. Thanks, Aaron Wood
Re: Review Request 54996: Fix SIGBUS crash on ARM64/AArch64.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54996/ --- (Updated Jan. 9, 2017, 4:42 p.m.) Review request for mesos and Jie Yu. Bugs: MESOS-6835 https://issues.apache.org/jira/browse/MESOS-6835 Repository: mesos Description --- Currently in the Linux launcher when the stack is allocated and prepared for a call to clone() it is not properly aligned. This is not an issue for x86 or x64 but for ARM64/AArch64 it is because of the requirement of having the stack aligned to a 16 byte boundary. While x86 and x64 also expect the stack to have a 16 byte aligned stack, it is not enforced. An explanation of the stack and requirements for ARM64 can be found here http://infocenter.arm.com/help/topic/com.arm.doc.ihi0055b/IHI0055B_aapcs64.pdf (specifically section 5.2.2.1 that says SP mod 16 = 0. The stack must be quad-word aligned.) Additionally, the way that the stack is currently allocated and passed to clone() accidentally chops off one entry, making a stack overflow using those missing 8 bytes a possibility. Fixing this while aligning the memory will fix both the issue of the stack overflow issue as well as the SIGBUS crash. We should also net better performance from having the stack aligned. Diffs (updated) - 3rdparty/stout/include/stout/os/linux.hpp 530f1a55b src/linux/ns.hpp 77789717e Diff: https://reviews.apache.org/r/54996/diff/ Testing --- Built Mesos from source and am currently running it in a test cluster. Launched both Docker and Mesos tasks via Marathon without any resulting crash (initial crash only happened with Mesos containerizer + linux_launcher, not with the posix_launcher). `make check -j4` also passes with no errors. Thanks, Aaron Wood
Re: Review Request 54996: Fix SIGBUS crash on ARM64/AArch64.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54996/ --- (Updated Jan. 9, 2017, 4:19 p.m.) Review request for mesos and Jie Yu. Bugs: MESOS-6835 https://issues.apache.org/jira/browse/MESOS-6835 Repository: mesos Description --- Currently in the Linux launcher when the stack is allocated and prepared for a call to clone() it is not properly aligned. This is not an issue for x86 or x64 but for ARM64/AArch64 it is because of the requirement of having the stack aligned to a 16 byte boundary. While x86 and x64 also expect the stack to have a 16 byte aligned stack, it is not enforced. An explanation of the stack and requirements for ARM64 can be found here http://infocenter.arm.com/help/topic/com.arm.doc.ihi0055b/IHI0055B_aapcs64.pdf (specifically section 5.2.2.1 that says SP mod 16 = 0. The stack must be quad-word aligned.) Additionally, the way that the stack is currently allocated and passed to clone() accidentally chops off one entry, making a stack overflow using those missing 8 bytes a possibility. Fixing this while aligning the memory will fix both the issue of the stack overflow issue as well as the SIGBUS crash. We should also net better performance from having the stack aligned. Diffs - 3rdparty/stout/include/stout/os/linux.hpp 530f1a55b src/linux/ns.hpp 77789717e Diff: https://reviews.apache.org/r/54996/diff/ Testing (updated) --- Built Mesos from source and am currently running it in a test cluster. Launched both Docker and Mesos tasks via Marathon without any resulting crash (initial crash only happened with Mesos containerizer + linux_launcher, not with the posix_launcher). `make check -j4` also passes with no errors. Thanks, Aaron Wood
Re: Review Request 54996: Fix SIGBUS crash on ARM64/AArch64.
> On Jan. 7, 2017, 6:42 p.m., Jie Yu wrote:
> > 3rdparty/stout/include/stout/os/linux.hpp, line 72
> > <https://reviews.apache.org/r/54996/diff/4/?file=1596764#file1596764line72>
> >
> > Please use errno error so that errno are in the error string.
> >
> > `return ErrnoError("Memory allocation failed");`
`posix_memalign` never sets `errno`. Shouldn't we just return `Error`?
- Aaron
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54996/#review160821
-------
On Jan. 4, 2017, 9:55 p.m., Aaron Wood wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54996/
> ---
>
> (Updated Jan. 4, 2017, 9:55 p.m.)
>
>
> Review request for mesos and Jie Yu.
>
>
> Bugs: MESOS-6835
> https://issues.apache.org/jira/browse/MESOS-6835
>
>
> Repository: mesos
>
>
> Description
> ---
>
> Currently in the Linux launcher when the stack is allocated and prepared for
> a call to clone() it is not properly aligned. This is not an issue for x86 or
> x64 but for ARM64/AArch64 it is because of the requirement of having the
> stack aligned to a 16 byte boundary. While x86 and x64 also expect the stack
> to have a 16 byte aligned stack, it is not enforced. An explanation of the
> stack and requirements for ARM64 can be found here
> http://infocenter.arm.com/help/topic/com.arm.doc.ihi0055b/IHI0055B_aapcs64.pdf
> (specifically section 5.2.2.1 that says SP mod 16 = 0. The stack must be
> quad-word aligned.)
>
> Additionally, the way that the stack is currently allocated and passed to
> clone() accidentally chops off one entry, making a stack overflow using those
> missing 8 bytes a possibility. Fixing this while aligning the memory will fix
> both the issue of the stack overflow issue as well as the SIGBUS crash. We
> should also net better performance from having the stack aligned.
>
>
> Diffs
> -
>
> 3rdparty/stout/include/stout/os/linux.hpp 530f1a55b
> src/linux/ns.hpp 77789717e
>
> Diff: https://reviews.apache.org/r/54996/diff/
>
>
> Testing
> ---
>
> Built Mesos from source and am currently running it in a test cluster.
> Launched both Docker and Mesos tasks via Marathon without any resulting crash
> (initial crash only happened with Mesos containerizer + linux_launcher, not
> with the posix_launcher).
>
>
> Thanks,
>
> Aaron Wood
>
>
Re: Review Request 54996: Fix SIGBUS crash on ARM64/AArch64.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54996/ --- (Updated Jan. 4, 2017, 9:55 p.m.) Review request for mesos and Jie Yu. Changes --- `posix_memalign` does not set `errno`. Bugs: MESOS-6835 https://issues.apache.org/jira/browse/MESOS-6835 Repository: mesos Description --- Currently in the Linux launcher when the stack is allocated and prepared for a call to clone() it is not properly aligned. This is not an issue for x86 or x64 but for ARM64/AArch64 it is because of the requirement of having the stack aligned to a 16 byte boundary. While x86 and x64 also expect the stack to have a 16 byte aligned stack, it is not enforced. An explanation of the stack and requirements for ARM64 can be found here http://infocenter.arm.com/help/topic/com.arm.doc.ihi0055b/IHI0055B_aapcs64.pdf (specifically section 5.2.2.1 that says SP mod 16 = 0. The stack must be quad-word aligned.) Additionally, the way that the stack is currently allocated and passed to clone() accidentally chops off one entry, making a stack overflow using those missing 8 bytes a possibility. Fixing this while aligning the memory will fix both the issue of the stack overflow issue as well as the SIGBUS crash. We should also net better performance from having the stack aligned. Diffs (updated) - 3rdparty/stout/include/stout/os/linux.hpp 530f1a55b src/linux/ns.hpp 77789717e Diff: https://reviews.apache.org/r/54996/diff/ Testing --- Built Mesos from source and am currently running it in a test cluster. Launched both Docker and Mesos tasks via Marathon without any resulting crash (initial crash only happened with Mesos containerizer + linux_launcher, not with the posix_launcher). Thanks, Aaron Wood
Re: Review Request 54996: Fix SIGBUS crash on ARM64/AArch64.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54996/#review160541 --- 3rdparty/stout/include/stout/os/linux.hpp (line 72) <https://reviews.apache.org/r/54996/#comment231691> Looks like `posix_memalign` never sets `errno`. Need to change this to return `Error`. - Aaron Wood On Jan. 4, 2017, 9:28 p.m., Aaron Wood wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/54996/ > --- > > (Updated Jan. 4, 2017, 9:28 p.m.) > > > Review request for mesos and Jie Yu. > > > Bugs: MESOS-6835 > https://issues.apache.org/jira/browse/MESOS-6835 > > > Repository: mesos > > > Description > --- > > Currently in the Linux launcher when the stack is allocated and prepared for > a call to clone() it is not properly aligned. This is not an issue for x86 or > x64 but for ARM64/AArch64 it is because of the requirement of having the > stack aligned to a 16 byte boundary. While x86 and x64 also expect the stack > to have a 16 byte aligned stack, it is not enforced. An explanation of the > stack and requirements for ARM64 can be found here > http://infocenter.arm.com/help/topic/com.arm.doc.ihi0055b/IHI0055B_aapcs64.pdf > (specifically section 5.2.2.1 that says SP mod 16 = 0. The stack must be > quad-word aligned.) > > Additionally, the way that the stack is currently allocated and passed to > clone() accidentally chops off one entry, making a stack overflow using those > missing 8 bytes a possibility. Fixing this while aligning the memory will fix > both the issue of the stack overflow issue as well as the SIGBUS crash. We > should also net better performance from having the stack aligned. > > > Diffs > - > > 3rdparty/stout/include/stout/os/linux.hpp 530f1a55b > src/linux/ns.hpp 77789717e > > Diff: https://reviews.apache.org/r/54996/diff/ > > > Testing > --- > > Built Mesos from source and am currently running it in a test cluster. > Launched both Docker and Mesos tasks via Marathon without any resulting crash > (initial crash only happened with Mesos containerizer + linux_launcher, not > with the posix_launcher). > > > Thanks, > > Aaron Wood > >
Re: Review Request 54996: Fix SIGBUS crash on ARM64/AArch64.
> On Jan. 4, 2017, 5:41 a.m., James Peach wrote:
> > 3rdparty/stout/include/stout/os/linux.hpp, line 120
> > <https://reviews.apache.org/r/54996/diff/2/?file=1596476#file1596476line120>
> >
> > Consider hoisting this into the `Stack` class:
> >
> > ```
> > void * Stack::start() {
> > return (uint8_t *)address + size;
> > }
> > ```
>
> Aaron Wood wrote:
> Don't we want to avoid C-style casts?
>
> James Peach wrote:
> Sure, you could `static_cast` here. Not much difference imho :)
>
> Aaron Wood wrote:
> True. I thought it was part of the Mesos style guide which is why I ask.
>
> Aaron Wood wrote:
> Dropping this to stick with `char*` instead.
Actually, let me mark this as fixed since I did add a `start()` method, it just
doesn't return a `void*`.
- Aaron
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54996/#review160464
---
On Jan. 4, 2017, 9:28 p.m., Aaron Wood wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54996/
> ---
>
> (Updated Jan. 4, 2017, 9:28 p.m.)
>
>
> Review request for mesos and Jie Yu.
>
>
> Bugs: MESOS-6835
> https://issues.apache.org/jira/browse/MESOS-6835
>
>
> Repository: mesos
>
>
> Description
> ---
>
> Currently in the Linux launcher when the stack is allocated and prepared for
> a call to clone() it is not properly aligned. This is not an issue for x86 or
> x64 but for ARM64/AArch64 it is because of the requirement of having the
> stack aligned to a 16 byte boundary. While x86 and x64 also expect the stack
> to have a 16 byte aligned stack, it is not enforced. An explanation of the
> stack and requirements for ARM64 can be found here
> http://infocenter.arm.com/help/topic/com.arm.doc.ihi0055b/IHI0055B_aapcs64.pdf
> (specifically section 5.2.2.1 that says SP mod 16 = 0. The stack must be
> quad-word aligned.)
>
> Additionally, the way that the stack is currently allocated and passed to
> clone() accidentally chops off one entry, making a stack overflow using those
> missing 8 bytes a possibility. Fixing this while aligning the memory will fix
> both the issue of the stack overflow issue as well as the SIGBUS crash. We
> should also net better performance from having the stack aligned.
>
>
> Diffs
> -
>
> 3rdparty/stout/include/stout/os/linux.hpp 530f1a55b
> src/linux/ns.hpp 77789717e
>
> Diff: https://reviews.apache.org/r/54996/diff/
>
>
> Testing
> ---
>
> Built Mesos from source and am currently running it in a test cluster.
> Launched both Docker and Mesos tasks via Marathon without any resulting crash
> (initial crash only happened with Mesos containerizer + linux_launcher, not
> with the posix_launcher).
>
>
> Thanks,
>
> Aaron Wood
>
>
Re: Review Request 54996: Fix SIGBUS crash on ARM64/AArch64.
> On Jan. 4, 2017, 5:41 a.m., James Peach wrote:
> > 3rdparty/stout/include/stout/os/linux.hpp, line 63
> > <https://reviews.apache.org/r/54996/diff/2/?file=1596476#file1596476line63>
> >
> > I'd recommend something like this:
> > ```
> > Try allocate(size_t nbytes) {
> > int error = ::posix_memalign(, os::getpagesize(), nbytes);
> > if (error) {
> > return ErrnoError(error);
> > }
> >
> > return Nothing();
> > }
> > ```
While my changes are not 100% aligned with what you suggested here, I did move
to using `getpagesize()`.
> On Jan. 4, 2017, 5:41 a.m., James Peach wrote:
> > 3rdparty/stout/include/stout/os/linux.hpp, line 60
> > <https://reviews.apache.org/r/54996/diff/2/?file=1596476#file1596476line60>
> >
> > I don't think that you need this static constructor. You can do
> > everything you need in `allocate`, which is them symmetric with
> > `deallocate`.
Dropping since Jie wants to keep the private constructor.
- Aaron
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54996/#review160464
---
On Jan. 4, 2017, 9:28 p.m., Aaron Wood wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54996/
> ---
>
> (Updated Jan. 4, 2017, 9:28 p.m.)
>
>
> Review request for mesos and Jie Yu.
>
>
> Bugs: MESOS-6835
> https://issues.apache.org/jira/browse/MESOS-6835
>
>
> Repository: mesos
>
>
> Description
> ---
>
> Currently in the Linux launcher when the stack is allocated and prepared for
> a call to clone() it is not properly aligned. This is not an issue for x86 or
> x64 but for ARM64/AArch64 it is because of the requirement of having the
> stack aligned to a 16 byte boundary. While x86 and x64 also expect the stack
> to have a 16 byte aligned stack, it is not enforced. An explanation of the
> stack and requirements for ARM64 can be found here
> http://infocenter.arm.com/help/topic/com.arm.doc.ihi0055b/IHI0055B_aapcs64.pdf
> (specifically section 5.2.2.1 that says SP mod 16 = 0. The stack must be
> quad-word aligned.)
>
> Additionally, the way that the stack is currently allocated and passed to
> clone() accidentally chops off one entry, making a stack overflow using those
> missing 8 bytes a possibility. Fixing this while aligning the memory will fix
> both the issue of the stack overflow issue as well as the SIGBUS crash. We
> should also net better performance from having the stack aligned.
>
>
> Diffs
> -
>
> 3rdparty/stout/include/stout/os/linux.hpp 530f1a55b
> src/linux/ns.hpp 77789717e
>
> Diff: https://reviews.apache.org/r/54996/diff/
>
>
> Testing
> ---
>
> Built Mesos from source and am currently running it in a test cluster.
> Launched both Docker and Mesos tasks via Marathon without any resulting crash
> (initial crash only happened with Mesos containerizer + linux_launcher, not
> with the posix_launcher).
>
>
> Thanks,
>
> Aaron Wood
>
>
Re: Review Request 54996: Fix SIGBUS crash on ARM64/AArch64.
> On Jan. 4, 2017, 5:41 a.m., James Peach wrote: > > 3rdparty/stout/include/stout/os/linux.hpp, line 82 > > <https://reviews.apache.org/r/54996/diff/2/?file=1596476#file1596476line82> > > > > You should explictly delete the copy constructors here to ensure the > > stack can't be leaked if the code changes: > > > > ``` > > Stack(const Stack&) = delete; > > Stack& operator=(const Stack&) = delete; > > ``` See comment below about why the copy constructor cannot be deleted in this case. - Aaron --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54996/#review160464 --- On Jan. 4, 2017, 9:28 p.m., Aaron Wood wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/54996/ > --- > > (Updated Jan. 4, 2017, 9:28 p.m.) > > > Review request for mesos and Jie Yu. > > > Bugs: MESOS-6835 > https://issues.apache.org/jira/browse/MESOS-6835 > > > Repository: mesos > > > Description > --- > > Currently in the Linux launcher when the stack is allocated and prepared for > a call to clone() it is not properly aligned. This is not an issue for x86 or > x64 but for ARM64/AArch64 it is because of the requirement of having the > stack aligned to a 16 byte boundary. While x86 and x64 also expect the stack > to have a 16 byte aligned stack, it is not enforced. An explanation of the > stack and requirements for ARM64 can be found here > http://infocenter.arm.com/help/topic/com.arm.doc.ihi0055b/IHI0055B_aapcs64.pdf > (specifically section 5.2.2.1 that says SP mod 16 = 0. The stack must be > quad-word aligned.) > > Additionally, the way that the stack is currently allocated and passed to > clone() accidentally chops off one entry, making a stack overflow using those > missing 8 bytes a possibility. Fixing this while aligning the memory will fix > both the issue of the stack overflow issue as well as the SIGBUS crash. We > should also net better performance from having the stack aligned. > > > Diffs > - > > 3rdparty/stout/include/stout/os/linux.hpp 530f1a55b > src/linux/ns.hpp 77789717e > > Diff: https://reviews.apache.org/r/54996/diff/ > > > Testing > --- > > Built Mesos from source and am currently running it in a test cluster. > Launched both Docker and Mesos tasks via Marathon without any resulting crash > (initial crash only happened with Mesos containerizer + linux_launcher, not > with the posix_launcher). > > > Thanks, > > Aaron Wood > >
Re: Review Request 54996: Fix SIGBUS crash on ARM64/AArch64.
> On Jan. 4, 2017, 5:41 a.m., James Peach wrote:
> > 3rdparty/stout/include/stout/os/linux.hpp, line 120
> > <https://reviews.apache.org/r/54996/diff/2/?file=1596476#file1596476line120>
> >
> > Consider hoisting this into the `Stack` class:
> >
> > ```
> > void * Stack::start() {
> > return (uint8_t *)address + size;
> > }
> > ```
>
> Aaron Wood wrote:
> Don't we want to avoid C-style casts?
>
> James Peach wrote:
> Sure, you could `static_cast` here. Not much difference imho :)
>
> Aaron Wood wrote:
> True. I thought it was part of the Mesos style guide which is why I ask.
Dropping this to stick with `char*` instead.
- Aaron
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54996/#review160464
---
On Jan. 4, 2017, 9:28 p.m., Aaron Wood wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54996/
> ---
>
> (Updated Jan. 4, 2017, 9:28 p.m.)
>
>
> Review request for mesos and Jie Yu.
>
>
> Bugs: MESOS-6835
> https://issues.apache.org/jira/browse/MESOS-6835
>
>
> Repository: mesos
>
>
> Description
> ---
>
> Currently in the Linux launcher when the stack is allocated and prepared for
> a call to clone() it is not properly aligned. This is not an issue for x86 or
> x64 but for ARM64/AArch64 it is because of the requirement of having the
> stack aligned to a 16 byte boundary. While x86 and x64 also expect the stack
> to have a 16 byte aligned stack, it is not enforced. An explanation of the
> stack and requirements for ARM64 can be found here
> http://infocenter.arm.com/help/topic/com.arm.doc.ihi0055b/IHI0055B_aapcs64.pdf
> (specifically section 5.2.2.1 that says SP mod 16 = 0. The stack must be
> quad-word aligned.)
>
> Additionally, the way that the stack is currently allocated and passed to
> clone() accidentally chops off one entry, making a stack overflow using those
> missing 8 bytes a possibility. Fixing this while aligning the memory will fix
> both the issue of the stack overflow issue as well as the SIGBUS crash. We
> should also net better performance from having the stack aligned.
>
>
> Diffs
> -
>
> 3rdparty/stout/include/stout/os/linux.hpp 530f1a55b
> src/linux/ns.hpp 77789717e
>
> Diff: https://reviews.apache.org/r/54996/diff/
>
>
> Testing
> ---
>
> Built Mesos from source and am currently running it in a test cluster.
> Launched both Docker and Mesos tasks via Marathon without any resulting crash
> (initial crash only happened with Mesos containerizer + linux_launcher, not
> with the posix_launcher).
>
>
> Thanks,
>
> Aaron Wood
>
>
Re: Review Request 54996: Fix SIGBUS crash on ARM64/AArch64.
> On Jan. 4, 2017, 5:58 p.m., Jie Yu wrote:
> > 3rdparty/stout/include/stout/os/linux.hpp, lines 57-60
> > <https://reviews.apache.org/r/54996/diff/2/?file=1596476#file1596476line57>
> >
> > I would actually suggest keep this `create` method, but move the
> > allocation logic in `create`.
> >
> > ```
> > static Try create(size_t size)
> > {
> > Stack stack(size);
> >
> > if (posix_memalign(...) != 0) {
> > return ErrnoError("Failed to allocate stack");
> > }
> >
> > return stack;
> > }
> > ```
> >
> > We can get rid of the `allocate` function. Once created, it's by
> > default allocated.
>
> Aaron Wood wrote:
> `address` and `size` would need to be static for this. That opens up
> another issue since all stacks would share the same data. Would it be better
> to get rid of the private constructor, delete the copy constructor and
> assignment, and just have `allocate()` and `deallocate()`?
>
> Jie Yu wrote:
> Why? you can do `stack.address` and `stack.size`. constructor is
> private, meaning that only 'create' can construct the Stack. Removing copy
> and assignment operator sounds good. Can you try that see if that compiles?
You're right, I was not thinking of using the local `stack` object directly.
Note that this only compiles with deleting the `=` operator since, as you had
said, the copy constructor `Try(const U& u)` is invoked underneath.
- Aaron
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54996/#review160513
---
On Jan. 4, 2017, 9:28 p.m., Aaron Wood wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54996/
> ---
>
> (Updated Jan. 4, 2017, 9:28 p.m.)
>
>
> Review request for mesos and Jie Yu.
>
>
> Bugs: MESOS-6835
> https://issues.apache.org/jira/browse/MESOS-6835
>
>
> Repository: mesos
>
>
> Description
> ---
>
> Currently in the Linux launcher when the stack is allocated and prepared for
> a call to clone() it is not properly aligned. This is not an issue for x86 or
> x64 but for ARM64/AArch64 it is because of the requirement of having the
> stack aligned to a 16 byte boundary. While x86 and x64 also expect the stack
> to have a 16 byte aligned stack, it is not enforced. An explanation of the
> stack and requirements for ARM64 can be found here
> http://infocenter.arm.com/help/topic/com.arm.doc.ihi0055b/IHI0055B_aapcs64.pdf
> (specifically section 5.2.2.1 that says SP mod 16 = 0. The stack must be
> quad-word aligned.)
>
> Additionally, the way that the stack is currently allocated and passed to
> clone() accidentally chops off one entry, making a stack overflow using those
> missing 8 bytes a possibility. Fixing this while aligning the memory will fix
> both the issue of the stack overflow issue as well as the SIGBUS crash. We
> should also net better performance from having the stack aligned.
>
>
> Diffs
> -
>
> 3rdparty/stout/include/stout/os/linux.hpp 530f1a55b
> src/linux/ns.hpp 77789717e
>
> Diff: https://reviews.apache.org/r/54996/diff/
>
>
> Testing
> ---
>
> Built Mesos from source and am currently running it in a test cluster.
> Launched both Docker and Mesos tasks via Marathon without any resulting crash
> (initial crash only happened with Mesos containerizer + linux_launcher, not
> with the posix_launcher).
>
>
> Thanks,
>
> Aaron Wood
>
>
Re: Review Request 54996: Fix SIGBUS crash on ARM64/AArch64.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54996/ --- (Updated Jan. 4, 2017, 9:28 p.m.) Review request for mesos and Jie Yu. Bugs: MESOS-6835 https://issues.apache.org/jira/browse/MESOS-6835 Repository: mesos Description --- Currently in the Linux launcher when the stack is allocated and prepared for a call to clone() it is not properly aligned. This is not an issue for x86 or x64 but for ARM64/AArch64 it is because of the requirement of having the stack aligned to a 16 byte boundary. While x86 and x64 also expect the stack to have a 16 byte aligned stack, it is not enforced. An explanation of the stack and requirements for ARM64 can be found here http://infocenter.arm.com/help/topic/com.arm.doc.ihi0055b/IHI0055B_aapcs64.pdf (specifically section 5.2.2.1 that says SP mod 16 = 0. The stack must be quad-word aligned.) Additionally, the way that the stack is currently allocated and passed to clone() accidentally chops off one entry, making a stack overflow using those missing 8 bytes a possibility. Fixing this while aligning the memory will fix both the issue of the stack overflow issue as well as the SIGBUS crash. We should also net better performance from having the stack aligned. Diffs (updated) - 3rdparty/stout/include/stout/os/linux.hpp 530f1a55b src/linux/ns.hpp 77789717e Diff: https://reviews.apache.org/r/54996/diff/ Testing --- Built Mesos from source and am currently running it in a test cluster. Launched both Docker and Mesos tasks via Marathon without any resulting crash (initial crash only happened with Mesos containerizer + linux_launcher, not with the posix_launcher). Thanks, Aaron Wood
Re: Review Request 54996: Fix SIGBUS crash on ARM64/AArch64.
> On Jan. 4, 2017, 5:58 p.m., Jie Yu wrote:
> > 3rdparty/stout/include/stout/os/linux.hpp, lines 57-60
> > <https://reviews.apache.org/r/54996/diff/2/?file=1596476#file1596476line57>
> >
> > I would actually suggest keep this `create` method, but move the
> > allocation logic in `create`.
> >
> > ```
> > static Try create(size_t size)
> > {
> > Stack stack(size);
> >
> > if (posix_memalign(...) != 0) {
> > return ErrnoError("Failed to allocate stack");
> > }
> >
> > return stack;
> > }
> > ```
> >
> > We can get rid of the `allocate` function. Once created, it's by
> > default allocated.
`address` and `size` would need to be static for this. That opens up another
issue since all stacks would share the same data. Would it be better to get rid
of the private constructor, delete the copy constructor and assignment, and
just have `allocate()` and `deallocate()`?
- Aaron
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54996/#review160513
---
On Jan. 4, 2017, 12:26 a.m., Aaron Wood wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54996/
> ---
>
> (Updated Jan. 4, 2017, 12:26 a.m.)
>
>
> Review request for mesos and Jie Yu.
>
>
> Bugs: MESOS-6835
> https://issues.apache.org/jira/browse/MESOS-6835
>
>
> Repository: mesos
>
>
> Description
> ---
>
> Currently in the Linux launcher when the stack is allocated and prepared for
> a call to clone() it is not properly aligned. This is not an issue for x86 or
> x64 but for ARM64/AArch64 it is because of the requirement of having the
> stack aligned to a 16 byte boundary. While x86 and x64 also expect the stack
> to have a 16 byte aligned stack, it is not enforced. An explanation of the
> stack and requirements for ARM64 can be found here
> http://infocenter.arm.com/help/topic/com.arm.doc.ihi0055b/IHI0055B_aapcs64.pdf
> (specifically section 5.2.2.1 that says SP mod 16 = 0. The stack must be
> quad-word aligned.)
>
> Additionally, the way that the stack is currently allocated and passed to
> clone() accidentally chops off one entry, making a stack overflow using those
> missing 8 bytes a possibility. Fixing this while aligning the memory will fix
> both the issue of the stack overflow issue as well as the SIGBUS crash. We
> should also net better performance from having the stack aligned.
>
>
> Diffs
> -
>
> 3rdparty/stout/include/stout/os/linux.hpp 530f1a55b
> src/linux/ns.hpp 77789717e
>
> Diff: https://reviews.apache.org/r/54996/diff/
>
>
> Testing
> ---
>
> Built Mesos from source and am currently running it in a test cluster.
> Launched both Docker and Mesos tasks via Marathon without any resulting crash
> (initial crash only happened with Mesos containerizer + linux_launcher, not
> with the posix_launcher).
>
>
> Thanks,
>
> Aaron Wood
>
>
Re: Review Request 54996: Fix SIGBUS crash on ARM64/AArch64.
> On Jan. 4, 2017, 5:41 a.m., James Peach wrote:
> > 3rdparty/stout/include/stout/os/linux.hpp, line 120
> > <https://reviews.apache.org/r/54996/diff/2/?file=1596476#file1596476line120>
> >
> > Consider hoisting this into the `Stack` class:
> >
> > ```
> > void * Stack::start() {
> > return (uint8_t *)address + size;
> > }
> > ```
>
> Aaron Wood wrote:
> Don't we want to avoid C-style casts?
>
> James Peach wrote:
> Sure, you could `static_cast` here. Not much difference imho :)
True. I thought it was part of the Mesos style guide which is why I ask.
- Aaron
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54996/#review160464
---
On Jan. 4, 2017, 12:26 a.m., Aaron Wood wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54996/
> ---
>
> (Updated Jan. 4, 2017, 12:26 a.m.)
>
>
> Review request for mesos and Jie Yu.
>
>
> Bugs: MESOS-6835
> https://issues.apache.org/jira/browse/MESOS-6835
>
>
> Repository: mesos
>
>
> Description
> ---
>
> Currently in the Linux launcher when the stack is allocated and prepared for
> a call to clone() it is not properly aligned. This is not an issue for x86 or
> x64 but for ARM64/AArch64 it is because of the requirement of having the
> stack aligned to a 16 byte boundary. While x86 and x64 also expect the stack
> to have a 16 byte aligned stack, it is not enforced. An explanation of the
> stack and requirements for ARM64 can be found here
> http://infocenter.arm.com/help/topic/com.arm.doc.ihi0055b/IHI0055B_aapcs64.pdf
> (specifically section 5.2.2.1 that says SP mod 16 = 0. The stack must be
> quad-word aligned.)
>
> Additionally, the way that the stack is currently allocated and passed to
> clone() accidentally chops off one entry, making a stack overflow using those
> missing 8 bytes a possibility. Fixing this while aligning the memory will fix
> both the issue of the stack overflow issue as well as the SIGBUS crash. We
> should also net better performance from having the stack aligned.
>
>
> Diffs
> -
>
> 3rdparty/stout/include/stout/os/linux.hpp 530f1a55b
> src/linux/ns.hpp 77789717e
>
> Diff: https://reviews.apache.org/r/54996/diff/
>
>
> Testing
> ---
>
> Built Mesos from source and am currently running it in a test cluster.
> Launched both Docker and Mesos tasks via Marathon without any resulting crash
> (initial crash only happened with Mesos containerizer + linux_launcher, not
> with the posix_launcher).
>
>
> Thanks,
>
> Aaron Wood
>
>
Re: Review Request 54996: Fix SIGBUS crash on ARM64/AArch64.
> On Jan. 4, 2017, 5:41 a.m., James Peach wrote:
> > 3rdparty/stout/include/stout/os/linux.hpp, line 120
> > <https://reviews.apache.org/r/54996/diff/2/?file=1596476#file1596476line120>
> >
> > Consider hoisting this into the `Stack` class:
> >
> > ```
> > void * Stack::start() {
> > return (uint8_t *)address + size;
> > }
> > ```
Don't we want to avoid C-style casts?
- Aaron
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54996/#review160464
-------
On Jan. 4, 2017, 12:26 a.m., Aaron Wood wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54996/
> ---
>
> (Updated Jan. 4, 2017, 12:26 a.m.)
>
>
> Review request for mesos and Jie Yu.
>
>
> Bugs: MESOS-6835
> https://issues.apache.org/jira/browse/MESOS-6835
>
>
> Repository: mesos
>
>
> Description
> ---
>
> Currently in the Linux launcher when the stack is allocated and prepared for
> a call to clone() it is not properly aligned. This is not an issue for x86 or
> x64 but for ARM64/AArch64 it is because of the requirement of having the
> stack aligned to a 16 byte boundary. While x86 and x64 also expect the stack
> to have a 16 byte aligned stack, it is not enforced. An explanation of the
> stack and requirements for ARM64 can be found here
> http://infocenter.arm.com/help/topic/com.arm.doc.ihi0055b/IHI0055B_aapcs64.pdf
> (specifically section 5.2.2.1 that says SP mod 16 = 0. The stack must be
> quad-word aligned.)
>
> Additionally, the way that the stack is currently allocated and passed to
> clone() accidentally chops off one entry, making a stack overflow using those
> missing 8 bytes a possibility. Fixing this while aligning the memory will fix
> both the issue of the stack overflow issue as well as the SIGBUS crash. We
> should also net better performance from having the stack aligned.
>
>
> Diffs
> -
>
> 3rdparty/stout/include/stout/os/linux.hpp 530f1a55b
> src/linux/ns.hpp 77789717e
>
> Diff: https://reviews.apache.org/r/54996/diff/
>
>
> Testing
> ---
>
> Built Mesos from source and am currently running it in a test cluster.
> Launched both Docker and Mesos tasks via Marathon without any resulting crash
> (initial crash only happened with Mesos containerizer + linux_launcher, not
> with the posix_launcher).
>
>
> Thanks,
>
> Aaron Wood
>
>
Re: Review Request 54996: Fix SIGBUS crash on ARM64/AArch64.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54996/#review160454 --- 3rdparty/stout/include/stout/os/linux.hpp (line 68) <https://reviews.apache.org/r/54996/#comment231598> Anyone see a way around this reinterpret_cast? - Aaron Wood On Jan. 4, 2017, 12:26 a.m., Aaron Wood wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/54996/ > --- > > (Updated Jan. 4, 2017, 12:26 a.m.) > > > Review request for mesos and Jie Yu. > > > Bugs: MESOS-6835 > https://issues.apache.org/jira/browse/MESOS-6835 > > > Repository: mesos > > > Description > --- > > Currently in the Linux launcher when the stack is allocated and prepared for > a call to clone() it is not properly aligned. This is not an issue for x86 or > x64 but for ARM64/AArch64 it is because of the requirement of having the > stack aligned to a 16 byte boundary. While x86 and x64 also expect the stack > to have a 16 byte aligned stack, it is not enforced. An explanation of the > stack and requirements for ARM64 can be found here > http://infocenter.arm.com/help/topic/com.arm.doc.ihi0055b/IHI0055B_aapcs64.pdf > (specifically section 5.2.2.1 that says SP mod 16 = 0. The stack must be > quad-word aligned.) > > Additionally, the way that the stack is currently allocated and passed to > clone() accidentally chops off one entry, making a stack overflow using those > missing 8 bytes a possibility. Fixing this while aligning the memory will fix > both the issue of the stack overflow issue as well as the SIGBUS crash. We > should also net better performance from having the stack aligned. > > > Diffs > - > > 3rdparty/stout/include/stout/os/linux.hpp 530f1a55b > src/linux/ns.hpp 77789717e > > Diff: https://reviews.apache.org/r/54996/diff/ > > > Testing > --- > > Built Mesos from source and am currently running it in a test cluster. > Launched both Docker and Mesos tasks via Marathon without any resulting crash > (initial crash only happened with Mesos containerizer + linux_launcher, not > with the posix_launcher). > > > Thanks, > > Aaron Wood > >
Re: Review Request 54996: Fix SIGBUS crash on ARM64/AArch64.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54996/ --- (Updated Jan. 4, 2017, 12:26 a.m.) Review request for mesos and Jie Yu. Changes --- Created stack class so that the logic for creating an aligned stack can be shared. Used this new class in another area of the codebase where a stack was being allocated. Bugs: MESOS-6835 https://issues.apache.org/jira/browse/MESOS-6835 Repository: mesos Description --- Currently in the Linux launcher when the stack is allocated and prepared for a call to clone() it is not properly aligned. This is not an issue for x86 or x64 but for ARM64/AArch64 it is because of the requirement of having the stack aligned to a 16 byte boundary. While x86 and x64 also expect the stack to have a 16 byte aligned stack, it is not enforced. An explanation of the stack and requirements for ARM64 can be found here http://infocenter.arm.com/help/topic/com.arm.doc.ihi0055b/IHI0055B_aapcs64.pdf (specifically section 5.2.2.1 that says SP mod 16 = 0. The stack must be quad-word aligned.) Additionally, the way that the stack is currently allocated and passed to clone() accidentally chops off one entry, making a stack overflow using those missing 8 bytes a possibility. Fixing this while aligning the memory will fix both the issue of the stack overflow issue as well as the SIGBUS crash. We should also net better performance from having the stack aligned. Diffs (updated) - 3rdparty/stout/include/stout/os/linux.hpp 530f1a55b src/linux/ns.hpp 77789717e Diff: https://reviews.apache.org/r/54996/diff/ Testing --- Built Mesos from source and am currently running it in a test cluster. Launched both Docker and Mesos tasks via Marathon without any resulting crash (initial crash only happened with Mesos containerizer + linux_launcher, not with the posix_launcher). Thanks, Aaron Wood
Re: Review Request 54996: Fix SIGBUS crash on ARM64/AArch64.
> On Dec. 27, 2016, 1:25 p.m., Till Toenshoff wrote:
> > 3rdparty/stout/include/stout/os/linux.hpp, line 81
> > <https://reviews.apache.org/r/54996/diff/1/?file=1591306#file1591306line81>
> >
> > As you have no further use for the returned value, I guess we could
> > simply spare the temporary and make it a bit more concise and also
> > C++'esque by prventing the C-style cast;
> >
> > ```
> > if (posix_memalign(static_cast<void**>(>address), 16,
> > stack->size) !=
> > 0) {
> > return -1;
> > }
> > ```
Sure, I will address this in combination with Jie's comments next week!
- Aaron
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54996/#review160159
---
On Dec. 29, 2016, 7:03 p.m., Aaron Wood wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54996/
> ---
>
> (Updated Dec. 29, 2016, 7:03 p.m.)
>
>
> Review request for mesos and Jie Yu.
>
>
> Bugs: MESOS-6835
> https://issues.apache.org/jira/browse/MESOS-6835
>
>
> Repository: mesos
>
>
> Description
> ---
>
> Currently in the Linux launcher when the stack is allocated and prepared for
> a call to clone() it is not properly aligned. This is not an issue for x86 or
> x64 but for ARM64/AArch64 it is because of the requirement of having the
> stack aligned to a 16 byte boundary. While x86 and x64 also expect the stack
> to have a 16 byte aligned stack, it is not enforced. An explanation of the
> stack and requirements for ARM64 can be found here
> http://infocenter.arm.com/help/topic/com.arm.doc.ihi0055b/IHI0055B_aapcs64.pdf
> (specifically section 5.2.2.1 that says SP mod 16 = 0. The stack must be
> quad-word aligned.)
>
> Additionally, the way that the stack is currently allocated and passed to
> clone() accidentally chops off one entry, making a stack overflow using those
> missing 8 bytes a possibility. Fixing this while aligning the memory will fix
> both the issue of the stack overflow issue as well as the SIGBUS crash. We
> should also net better performance from having the stack aligned.
>
>
> Diffs
> -
>
> 3rdparty/stout/include/stout/os/linux.hpp 530f1a55b
>
> Diff: https://reviews.apache.org/r/54996/diff/
>
>
> Testing
> ---
>
> Built Mesos from source and am currently running it in a test cluster.
> Launched both Docker and Mesos tasks via Marathon without any resulting crash
> (initial crash only happened with Mesos containerizer + linux_launcher, not
> with the posix_launcher).
>
>
> Thanks,
>
> Aaron Wood
>
>
Re: Review Request 54996: Fix SIGBUS crash on ARM64/AArch64.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54996/ --- (Updated Dec. 29, 2016, 7:03 p.m.) Review request for mesos and Jie Yu. Bugs: MESOS-6835 https://issues.apache.org/jira/browse/MESOS-6835 Repository: mesos Description (updated) --- Currently in the Linux launcher when the stack is allocated and prepared for a call to clone() it is not properly aligned. This is not an issue for x86 or x64 but for ARM64/AArch64 it is because of the requirement of having the stack aligned to a 16 byte boundary. While x86 and x64 also expect the stack to have a 16 byte aligned stack, it is not enforced. An explanation of the stack and requirements for ARM64 can be found here http://infocenter.arm.com/help/topic/com.arm.doc.ihi0055b/IHI0055B_aapcs64.pdf (specifically section 5.2.2.1 that says SP mod 16 = 0. The stack must be quad-word aligned.) Additionally, the way that the stack is currently allocated and passed to clone() accidentally chops off one entry, making a stack overflow using those missing 8 bytes a possibility. Fixing this while aligning the memory will fix both the issue of the stack overflow issue as well as the SIGBUS crash. We should also net better performance from having the stack aligned. Diffs - 3rdparty/stout/include/stout/os/linux.hpp 530f1a55b Diff: https://reviews.apache.org/r/54996/diff/ Testing --- Built Mesos from source and am currently running it in a test cluster. Launched both Docker and Mesos tasks via Marathon without any resulting crash (initial crash only happened with Mesos containerizer + linux_launcher, not with the posix_launcher). Thanks, Aaron Wood
Re: Review Request 54996: Fix SIGBUS crash on ARM64/AArch64.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54996/ --- (Updated Dec. 22, 2016, 9:24 p.m.) Review request for mesos and Jie Yu. Changes --- Referenced ARM documentation for the required stack alignment. Bugs: MESOS-6835 https://issues.apache.org/jira/browse/MESOS-6835 Repository: mesos Description (updated) --- Currently in the Linux launcher when the stack is allocated and prepared for a call to clone() it is not properly aligned. This is not an issue for x86 or x64 but for ARM64/AArch64 it is because of the requirement of having the stack aligned to a 16 byte boundary. While x86 and x64 also expect the stack to have a 16 byte aligned stack, it is not enforced. An explanation of the stack and requirements for ARM64 can be found here http://infocenter.arm.com/help/topic/com.arm.doc.ihi0055b/IHI0055B_aapcs64.pdf (specifically section 5.2.2.1 that says SP mod 16 = 0. The stack must be quad-word aligned.) Additionally, the way that the stack is currently allocated and passed to clone() accidentally chops off one entry, making a stack overflow using those missing 8 bytes a possibility. Fixing this while aligning the memory will fix both the issue of the stack overflow issue as well as the SIGBUS crash. Diffs - 3rdparty/stout/include/stout/os/linux.hpp 530f1a55b Diff: https://reviews.apache.org/r/54996/diff/ Testing --- Built Mesos from source and am currently running it in a test cluster. Launched both Docker and Mesos tasks via Marathon without any resulting crash (initial crash only happened with Mesos containerizer + linux_launcher, not with the posix_launcher). Thanks, Aaron Wood
Re: Review Request 54993: Modify LevelDB patch to add ARM64/AArch64 support.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54993/ --- (Updated Dec. 22, 2016, 9:10 p.m.) Review request for mesos and Jie Yu. Bugs: MESOS-6834 https://issues.apache.org/jira/browse/MESOS-6834 Repository: mesos Description --- Mesos will not compile on ARM64/AArch64 without a patch to the version of LevelDB that is used within Mesos. While the fix is already in newer versions of LevelDB it's not something that Mesos pulls down. The main issue is that the AtomicPointer header needs to be aware of other architectures to provide its functionality to those architectures. Diffs - 3rdparty/leveldb-1.4.patch b899f0141 Diff: https://reviews.apache.org/r/54993/diff/ Testing (updated) --- Compiled Mesos on ARM64 with no failures. Mesos also properly starts and successfully runs/launches tasks with the exception of a crash when using the linux_launcher and Mesos containers. That fix is addressed in https://reviews.apache.org/r/54996/ Thanks, Aaron Wood
Review Request 54996: Fix SIGBUS crash on ARM64/AArch64.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54996/ --- Review request for mesos and Jie Yu. Bugs: MESOS-6835 https://issues.apache.org/jira/browse/MESOS-6835 Repository: mesos Description --- Currently in the Linux launcher when the stack is allocated and prepared for a call to clone() it is not properly aligned. This is not an issue for x86 or x64 but for ARM64/AArch64 it is because of the requirement of having the stack aligned to a 16 byte boundary. While x86 and x64 also expect the stack to have a 16 byte aligned stack, it is not enforced. Additionally, the way that the stack is currently allocated and passed to clone() accidentally chops off one entry, making a stack overflow using those missing 8 bytes a possibility. Fixing this while aligning the memory will fix both the issue of the stack overflow issue as well as the SIGBUS crash. Diffs - 3rdparty/stout/include/stout/os/linux.hpp 530f1a55b Diff: https://reviews.apache.org/r/54996/diff/ Testing --- Built Mesos from source and am currently running it in a test cluster. Launched both Docker and Mesos tasks via Marathon without any resulting crash (initial crash only happened with Mesos containerizer + linux_launcher, not with the posix_launcher). Thanks, Aaron Wood
Re: Review Request 54993: Modify LevelDB patch to add ARM64/AArch64 support.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54993/ --- (Updated Dec. 22, 2016, 9:02 p.m.) Review request for mesos and Jie Yu. Bugs: MESOS-6834 https://issues.apache.org/jira/browse/MESOS-6834 Repository: mesos Description --- Mesos will not compile on ARM64/AArch64 without a patch to the version of LevelDB that is used within Mesos. While the fix is already in newer versions of LevelDB it's not something that Mesos pulls down. The main issue is that the AtomicPointer header needs to be aware of other architectures to provide its functionality to those architectures. Diffs - 3rdparty/leveldb-1.4.patch b899f0141 Diff: https://reviews.apache.org/r/54993/diff/ Testing (updated) --- Compiled Mesos on ARM64 with no failures. Mesos also properly starts and successfully runs with the exception of an issue with Mesos containers (addressed in https://reviews.apache.org/r/54996/). Thanks, Aaron Wood
Re: Review Request 54993: Modify LevelDB patch to add ARM64/AArch64 support.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54993/ --- (Updated Dec. 22, 2016, 9 p.m.) Review request for mesos and Jie Yu. Bugs: MESOS-6834 https://issues.apache.org/jira/browse/MESOS-6834 Repository: mesos Description (updated) --- Mesos will not compile on ARM64/AArch64 without a patch to the version of LevelDB that is used within Mesos. While the fix is already in newer versions of LevelDB it's not something that Mesos pulls down. The main issue is that the AtomicPointer header needs to be aware of other architectures to provide its functionality to those architectures. Diffs - 3rdparty/leveldb-1.4.patch b899f0141 Diff: https://reviews.apache.org/r/54993/diff/ Testing --- Compiled Mesos on ARM64 with no failures. Mesos also properly starts and successfully runs with the exception of an issue with Mesos containers (addressed in a separate review). Thanks, Aaron Wood
Review Request 54993: Modify LevelDB patch to add ARM64/AArch64 support.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54993/ --- Review request for mesos and Jie Yu. Bugs: MESOS-6834 https://issues.apache.org/jira/browse/MESOS-6834 Repository: mesos Description --- Mesos will not compile on ARM64/AArch64 without a patch to the version of LevelDB that is used within Mesos. While the fix is already in newer versions of LevelDB it's not something that Mesos pulls down. The main issue is that the AtomicPointer header needs to be aware of other architectures to provide its functionality to those architectures. Diffs - 3rdparty/leveldb-1.4.patch b899f0141 Diff: https://reviews.apache.org/r/54993/diff/ Testing --- Compiled Mesos on ARM64 with no failures. Mesos also properly starts and successfully runs with the exception of an issue with Mesos containers (addressed in a separate review). Thanks, Aaron Wood
Re: Review Request 54953: Set -pie conditionally on whether hardening is enabled or not.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54953/ --- (Updated Dec. 22, 2016, 12:30 a.m.) Review request for mesos and Michael Park. Changes --- Add on to `AM_LDFLAGS`. Bugs: MESOS-6830 https://issues.apache.org/jira/browse/MESOS-6830 Repository: mesos Description --- The gold linker complains about using -pie without -fPIC which causes the build to fail right before the end. This makes sure both -pie and -fPIC/-fPIE are enabled/disabled based on the setting for enabling/disabling hardening. Diffs (updated) - src/Makefile.am abcf7eed7 Diff: https://reviews.apache.org/r/54953/diff/ Testing --- ../configure --disable-python --disable-java && make ../configure --disable-python --disable-java --disable-hardening && make Thanks, Aaron Wood
Re: Review Request 54953: Set -pie conditionally on whether hardening is enabled or not.
> On Dec. 21, 2016, 11:57 p.m., Michael Park wrote: > > src/Makefile.am, line 112 > > <https://reviews.apache.org/r/54953/diff/1/?file=1590647#file1590647line112> > > > > Actually, I think we want to `+=` here instead? You're right, good catch. - Aaron --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54953/#review159886 --- On Dec. 21, 2016, 10:50 p.m., Aaron Wood wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/54953/ > --- > > (Updated Dec. 21, 2016, 10:50 p.m.) > > > Review request for mesos and Michael Park. > > > Bugs: MESOS-6830 > https://issues.apache.org/jira/browse/MESOS-6830 > > > Repository: mesos > > > Description > --- > > The gold linker complains about using -pie without -fPIC which causes the > build to fail right before the end. This makes sure both -pie and -fPIC/-fPIE > are enabled/disabled based on the setting for enabling/disabling hardening. > > > Diffs > - > > src/Makefile.am abcf7eed7 > > Diff: https://reviews.apache.org/r/54953/diff/ > > > Testing > --- > > ../configure --disable-python --disable-java && make > ../configure --disable-python --disable-java --disable-hardening && make > > > Thanks, > > Aaron Wood > >
Review Request 54953: Set -pie conditionally on whether hardening is enabled or not.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54953/ --- Review request for mesos and Michael Park. Bugs: MESOS-6830 https://issues.apache.org/jira/browse/MESOS-6830 Repository: mesos Description --- The gold linker complains about using -pie without -fPIC which causes the build to fail right before the end. This makes sure both -pie and -fPIC/-fPIE are enabled/disabled based on the setting for enabling/disabling hardening. Diffs - src/Makefile.am abcf7eed7 Diff: https://reviews.apache.org/r/54953/diff/ Testing --- ../configure --disable-python --disable-java && make ../configure --disable-python --disable-java --disable-hardening && make Thanks, Aaron Wood
Review Request 54951: Remove the use of FORTIFY_SOURCE from stout.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54951/ --- Review request for mesos and Michael Park. Bugs: MESOS-6829 https://issues.apache.org/jira/browse/MESOS-6829 Repository: mesos Description --- FORTIFY_SOURCE is no longer used when hardening. This is to prevent the warning that some versions of gcc/libc throw when FORTIFY_SOURCE is used without optimizations. The warning turns into an error via -Werror which is applied to MESOS_CPPFLAGS thus failing the whole build. Diffs - 3rdparty/stout/Makefile.am 2d27da7e6 Diff: https://reviews.apache.org/r/54951/diff/ Testing --- Build all of Mesos from source. Thanks, Aaron Wood
