Re: Review Request 63269: Windows: Added `os::set_job_cpu_limit` to stout.

[Andrew Schwartzmeyer]

> On Nov. 10, 2017, 5:14 p.m., Akash Gupta wrote:
> > 3rdparty/stout/include/stout/windows/os.hpp
> > Lines 745 (patched)
> > 
> >
> > Just realized this, but I don't think function works if your machine 
> > has > 64 CPUs. It calls GetSystemInfo, which returns the cpus of the 
> > "current" group, which is 64 max. Call 
> > GetMaximumProcessorCount(ALL_PROCESSOR_GROUPS) to get the right number. 
> > There was a bug fix in python with a similar issue to this: 
> > https://github.com/python/cpython/commit/c67bae04780f9d7590f9f91b4ee5f31c5d75b3c3
> 
> Andrew Schwartzmeyer wrote:
> That's a good point, but it's separate issue I'll file.

https://issues.apache.org/jira/browse/MESOS-8216


- Andrew


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


On Nov. 2, 2017, 1:39 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63269/
> ---
> 
> (Updated Nov. 2, 2017, 1: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 is used to set a hard cap on the CPU usage for a job object.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/windows/os.hpp 
> 09ddec6d69472cd13b453fe1a77fdbe343fc23c8 
> 
> 
> Diff: https://reviews.apache.org/r/63269/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 63269: Windows: Added `os::set_job_cpu_limit` to stout.

[Andrew Schwartzmeyer]

> On Nov. 10, 2017, 5:14 p.m., Akash Gupta wrote:
> > 3rdparty/stout/include/stout/windows/os.hpp
> > Lines 745 (patched)
> > 
> >
> > Just realized this, but I don't think function works if your machine 
> > has > 64 CPUs. It calls GetSystemInfo, which returns the cpus of the 
> > "current" group, which is 64 max. Call 
> > GetMaximumProcessorCount(ALL_PROCESSOR_GROUPS) to get the right number. 
> > There was a bug fix in python with a similar issue to this: 
> > https://github.com/python/cpython/commit/c67bae04780f9d7590f9f91b4ee5f31c5d75b3c3

That's a good point, but it's separate issue I'll file.


- Andrew


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


On Nov. 2, 2017, 1:39 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63269/
> ---
> 
> (Updated Nov. 2, 2017, 1: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 is used to set a hard cap on the CPU usage for a job object.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/windows/os.hpp 
> 09ddec6d69472cd13b453fe1a77fdbe343fc23c8 
> 
> 
> Diff: https://reviews.apache.org/r/63269/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 63269: Windows: Added `os::set_job_cpu_limit` to stout.

[Andrew Schwartzmeyer]

> On Nov. 10, 2017, 4:56 p.m., Akash Gupta wrote:
> > 3rdparty/stout/include/stout/windows/os.hpp
> > Lines 760 (patched)
> > 
> >
> > small nit, but for consistency, change false to FALSE.

`FALSE` is used for the Windows APIs, and this is a `stout` API which takes 
`false`.


- Andrew


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


On Nov. 2, 2017, 1:39 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63269/
> ---
> 
> (Updated Nov. 2, 2017, 1: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 is used to set a hard cap on the CPU usage for a job object.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/windows/os.hpp 
> 09ddec6d69472cd13b453fe1a77fdbe343fc23c8 
> 
> 
> Diff: https://reviews.apache.org/r/63269/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 63269: Windows: Added `os::set_job_cpu_limit` to stout.

[Akash Gupta]

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




3rdparty/stout/include/stout/windows/os.hpp
Lines 745 (patched)


Just realized this, but I don't think function works if your machine has > 
64 CPUs. It calls GetSystemInfo, which returns the cpus of the "current" group, 
which is 64 max. Call GetMaximumProcessorCount(ALL_PROCESSOR_GROUPS) to get the 
right number. There was a bug fix in python with a similar issue to this: 
https://github.com/python/cpython/commit/c67bae04780f9d7590f9f91b4ee5f31c5d75b3c3


- Akash Gupta


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/63269/
> ---
> 
> (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 is used to set a hard cap on the CPU usage for a job object.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/windows/os.hpp 
> 09ddec6d69472cd13b453fe1a77fdbe343fc23c8 
> 
> 
> Diff: https://reviews.apache.org/r/63269/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 63269: Windows: Added `os::set_job_cpu_limit` to stout.

[Akash Gupta]

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


Ship it!




Ship It!

- Akash Gupta


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/63269/
> ---
> 
> (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 is used to set a hard cap on the CPU usage for a job object.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/windows/os.hpp 
> 09ddec6d69472cd13b453fe1a77fdbe343fc23c8 
> 
> 
> Diff: https://reviews.apache.org/r/63269/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 63269: Windows: Added `os::set_job_cpu_limit` to stout.

[Akash Gupta]

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




3rdparty/stout/include/stout/windows/os.hpp
Lines 760 (patched)


small nit, but for consistency, change false to FALSE.


- Akash Gupta


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/63269/
> ---
> 
> (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 is used to set a hard cap on the CPU usage for a job object.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/windows/os.hpp 
> 09ddec6d69472cd13b453fe1a77fdbe343fc23c8 
> 
> 
> Diff: https://reviews.apache.org/r/63269/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 63269: Windows: Added `os::set_job_cpu_limit` to stout.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 63269 was successfully built and tested.

Reviews applied: `['63268', '63269']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/63269

- Mesos Reviewbot Windows


On Oct. 26, 2017, 4:29 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63269/
> ---
> 
> (Updated Oct. 26, 2017, 4:29 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Jeff Coffler, Jie Yu, John Kordich, 
> and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is used to set a hard cap on the CPU usage for a job object.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/windows/os.hpp 
> 09ddec6d69472cd13b453fe1a77fdbe343fc23c8 
> 
> 
> Diff: https://reviews.apache.org/r/63269/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Review Request 63269: Windows: Added `os::set_job_cpu_limit` to stout.

[Andrew Schwartzmeyer]

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

Review request for mesos, Akash Gupta, Jeff Coffler, Jie Yu, John Kordich, and 
Joseph Wu.


Repository: mesos


Description
---

This is used to set a hard cap on the CPU usage for a job object.


Diffs
-

  3rdparty/stout/include/stout/windows/os.hpp 
09ddec6d69472cd13b453fe1a77fdbe343fc23c8 


Diff: https://reviews.apache.org/r/63269/diff/1/


Testing
---


Thanks,

Andrew Schwartzmeyer