Re: Review Request 65891: Windows: Fixed bug with CPU isolator not checking the max boundary.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65891/#review198582 --- Patch looks great! Reviews applied: [65890, 65891] Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh - Mesos Reviewbot On March 2, 2018, 11:21 p.m., Andrew Schwartzmeyer wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/65891/ > --- > > (Updated March 2, 2018, 11:21 p.m.) > > > Review request for mesos, Akash Gupta and Joseph Wu. > > > Bugs: MESOS-8631 > https://issues.apache.org/jira/browse/MESOS-8631 > > > Repository: mesos > > > Description > --- > > The `set_job_cpu_limit` function does not sanitize its input, e.g. > `cpus`, causing a bug when more CPUs were requested than exist on the > machine. > > For instance, in Mesos it was not possible to start a single task > which consumed every CPU, because the executor overcommit would cause > the task to ask for 0.1 more CPUs than available. So on a quad-core > machine, it'd ask for 4 CPUs, but that would cause the limiter to set > a limit at 4.1 CPUs, which exceeds the maximum allowed, causing an > "Invalid parameter" error when setting the CPU limit. We can avoid > this by ensuring the `cpu_rate` value is in the range of [1, 1], > regardless of the input. > > > Diffs > - > > 3rdparty/stout/include/stout/windows/os.hpp > 545a0052c87652ec35adf12bb184fd35f1238977 > > > Diff: https://reviews.apache.org/r/65891/diff/1/ > > > Testing > --- > > Tested manually and can successfully now deploy a 12 CPU task on a 12 core > machine. > > Will file an issue to add appropriate unit tests. > > > Thanks, > > Andrew Schwartzmeyer > >
Re: Review Request 65891: Windows: Fixed bug with CPU isolator not checking the max boundary.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65891/#review198576 --- PASS: Mesos patch 65891 was successfully built and tested. Reviews applied: `['65890', '65891']` All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/65891 - Mesos Reviewbot Windows On March 2, 2018, 3:21 p.m., Andrew Schwartzmeyer wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/65891/ > --- > > (Updated March 2, 2018, 3:21 p.m.) > > > Review request for mesos, Akash Gupta and Joseph Wu. > > > Bugs: MESOS-8631 > https://issues.apache.org/jira/browse/MESOS-8631 > > > Repository: mesos > > > Description > --- > > The `set_job_cpu_limit` function does not sanitize its input, e.g. > `cpus`, causing a bug when more CPUs were requested than exist on the > machine. > > For instance, in Mesos it was not possible to start a single task > which consumed every CPU, because the executor overcommit would cause > the task to ask for 0.1 more CPUs than available. So on a quad-core > machine, it'd ask for 4 CPUs, but that would cause the limiter to set > a limit at 4.1 CPUs, which exceeds the maximum allowed, causing an > "Invalid parameter" error when setting the CPU limit. We can avoid > this by ensuring the `cpu_rate` value is in the range of [1, 1], > regardless of the input. > > > Diffs > - > > 3rdparty/stout/include/stout/windows/os.hpp > 545a0052c87652ec35adf12bb184fd35f1238977 > > > Diff: https://reviews.apache.org/r/65891/diff/1/ > > > Testing > --- > > Tested manually and can successfully now deploy a 12 CPU task on a 12 core > machine. > > Will file an issue to add appropriate unit tests. > > > Thanks, > > Andrew Schwartzmeyer > >
Re: Review Request 65891: Windows: Fixed bug with CPU isolator not checking the max boundary.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65891/#review198574 --- Ship it! LGTM. Would want confirmation from Zihao - Akash Gupta On March 2, 2018, 11:21 p.m., Andrew Schwartzmeyer wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/65891/ > --- > > (Updated March 2, 2018, 11:21 p.m.) > > > Review request for mesos, Akash Gupta and Joseph Wu. > > > Bugs: MESOS-8631 > https://issues.apache.org/jira/browse/MESOS-8631 > > > Repository: mesos > > > Description > --- > > The `set_job_cpu_limit` function does not sanitize its input, e.g. > `cpus`, causing a bug when more CPUs were requested than exist on the > machine. > > For instance, in Mesos it was not possible to start a single task > which consumed every CPU, because the executor overcommit would cause > the task to ask for 0.1 more CPUs than available. So on a quad-core > machine, it'd ask for 4 CPUs, but that would cause the limiter to set > a limit at 4.1 CPUs, which exceeds the maximum allowed, causing an > "Invalid parameter" error when setting the CPU limit. We can avoid > this by ensuring the `cpu_rate` value is in the range of [1, 1], > regardless of the input. > > > Diffs > - > > 3rdparty/stout/include/stout/windows/os.hpp > 545a0052c87652ec35adf12bb184fd35f1238977 > > > Diff: https://reviews.apache.org/r/65891/diff/1/ > > > Testing > --- > > Tested manually and can successfully now deploy a 12 CPU task on a 12 core > machine. > > Will file an issue to add appropriate unit tests. > > > Thanks, > > Andrew Schwartzmeyer > >
Re: Review Request 65891: Windows: Fixed bug with CPU isolator not checking the max boundary.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65891/ --- (Updated March 2, 2018, 3:21 p.m.) Review request for mesos, Akash Gupta and Joseph Wu. Changes --- Add bug. Bugs: MESOS-8631 https://issues.apache.org/jira/browse/MESOS-8631 Repository: mesos Description --- The `set_job_cpu_limit` function does not sanitize its input, e.g. `cpus`, causing a bug when more CPUs were requested than exist on the machine. For instance, in Mesos it was not possible to start a single task which consumed every CPU, because the executor overcommit would cause the task to ask for 0.1 more CPUs than available. So on a quad-core machine, it'd ask for 4 CPUs, but that would cause the limiter to set a limit at 4.1 CPUs, which exceeds the maximum allowed, causing an "Invalid parameter" error when setting the CPU limit. We can avoid this by ensuring the `cpu_rate` value is in the range of [1, 1], regardless of the input. Diffs - 3rdparty/stout/include/stout/windows/os.hpp 545a0052c87652ec35adf12bb184fd35f1238977 Diff: https://reviews.apache.org/r/65891/diff/1/ Testing --- Tested manually and can successfully now deploy a 12 CPU task on a 12 core machine. Will file an issue to add appropriate unit tests. Thanks, Andrew Schwartzmeyer
