Re: Review Request 43144: Added env var to set upper bound on number of libprocess worker threads.

[Joris Van Remoortere]

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


Ship it!




Ship It!

- Joris Van Remoortere


On March 30, 2016, 5:12 p.m., Maged Michael wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43144/
> ---
> 
> (Updated March 30, 2016, 5:12 p.m.)
> 
> 
> Review request for mesos, Guangya Liu, Joris Van Remoortere, Klaus Ma, and 
> Qian Zhang.
> 
> 
> Bugs: MESOS-4353
> https://issues.apache.org/jira/browse/MESOS-4353
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added env var to set upper bound on number of libprocess worker threads.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp feaffa4 
> 
> Diff: https://reviews.apache.org/r/43144/diff/
> 
> 
> Testing
> ---
> 
> Passed make check on x86_64 Ubuntu 14.04.
> 
> Invocations of mesos-master, mesos-slave, and mesos-tests with the 
> LIBPROCESS_MAX_WORKER_THREADS set to the values "1", "7", "9", "1000", "0", 
> "-1", and "abc" on a 4-core x86_64 Ubuntu 14.04 system. The results were as 
> expected. 1, 7, 8, 8, 8, 8, and 8 worker threads were created, respectively. 
> The last 3 tests generated warnings.
> 
> 
> Thanks,
> 
> Maged Michael
> 
>

Re: Review Request 43144: Added env var to set upper bound on number of libprocess worker threads.

[Maged Michael]

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

(Updated March 30, 2016, 5:12 p.m.)


Review request for mesos, Guangya Liu, Joris Van Remoortere, Klaus Ma, and Qian 
Zhang.


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


Repository: mesos


Description
---

Added env var to set upper bound on number of libprocess worker threads.


Diffs (updated)
-

  3rdparty/libprocess/src/process.cpp feaffa4 

Diff: https://reviews.apache.org/r/43144/diff/


Testing
---

Passed make check on x86_64 Ubuntu 14.04.

Invocations of mesos-master, mesos-slave, and mesos-tests with the 
LIBPROCESS_MAX_WORKER_THREADS set to the values "1", "7", "9", "1000", "0", 
"-1", and "abc" on a 4-core x86_64 Ubuntu 14.04 system. The results were as 
expected. 1, 7, 8, 8, 8, 8, and 8 worker threads were created, respectively. 
The last 3 tests generated warnings.


Thanks,

Maged Michael

Re: Review Request 43144: Added env var to set upper bound on number of libprocess worker threads.

[Maged Michael]

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

(Updated March 30, 2016, 5:10 p.m.)


Review request for mesos, Guangya Liu, Joris Van Remoortere, Klaus Ma, and Qian 
Zhang.


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


Repository: mesos


Description
---

Added env var to set upper bound on number of libprocess worker threads.


Diffs (updated)
-

  3rdparty/libprocess/src/process.cpp feaffa4 
  docs/configuration.md 9ad0c2a 

Diff: https://reviews.apache.org/r/43144/diff/


Testing
---

Passed make check on x86_64 Ubuntu 14.04.

Invocations of mesos-master, mesos-slave, and mesos-tests with the 
LIBPROCESS_MAX_WORKER_THREADS set to the values "1", "7", "9", "1000", "0", 
"-1", and "abc" on a 4-core x86_64 Ubuntu 14.04 system. The results were as 
expected. 1, 7, 8, 8, 8, 8, and 8 worker threads were created, respectively. 
The last 3 tests generated warnings.


Thanks,

Maged Michael

Re: Review Request 43144: Added env var to set upper bound on number of libprocess worker threads.

[Joris Van Remoortere]

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



Can you please split the review between the code update and the docs? The git 
commit hooks prevent committing accross projects (in this case Libprocess, 
Mesos).
I would do it for you, but since this is your first commit I don't have an 
author to reference for committing on your behalf ;-)


3rdparty/libprocess/src/process.cpp (line 2210)


s/system defined/default



3rdparty/libprocess/src/process.cpp (line 2217)


/system defined/default



3rdparty/libprocess/src/process.cpp (line )


New line after closing brace `}`.
space between operands `num_worker_threads + 1` (I know it was incorrect 
before)


- Joris Van Remoortere


On March 30, 2016, 12:07 p.m., Maged Michael wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43144/
> ---
> 
> (Updated March 30, 2016, 12:07 p.m.)
> 
> 
> Review request for mesos, Guangya Liu, Joris Van Remoortere, Klaus Ma, and 
> Qian Zhang.
> 
> 
> Bugs: MESOS-4353
> https://issues.apache.org/jira/browse/MESOS-4353
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added env var to set upper bound on number of libprocess worker threads.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp feaffa4 
>   docs/configuration.md 9ad0c2a 
> 
> Diff: https://reviews.apache.org/r/43144/diff/
> 
> 
> Testing
> ---
> 
> Passed make check on x86_64 Ubuntu 14.04.
> 
> Invocations of mesos-master, mesos-slave, and mesos-tests with the 
> LIBPROCESS_MAX_WORKER_THREADS set to the values "1", "7", "9", "1000", "0", 
> "-1", and "abc" on a 4-core x86_64 Ubuntu 14.04 system. The results were as 
> expected. 1, 7, 8, 8, 8, 8, and 8 worker threads were created, respectively. 
> The last 3 tests generated warnings.
> 
> 
> Thanks,
> 
> Maged Michael
> 
>

Re: Review Request 43144: Added env var to set upper bound on number of libprocess worker threads.

[Maged Michael]

> On March 29, 2016, 12:52 p.m., Joris Van Remoortere wrote:
> > 3rdparty/libprocess/src/process.cpp, line 2193
> > 
> >
> > since this value will not longer always map to the number of `cpus`, 
> > can we rename this to something more meaningful such as 
> > `num_worker_threads`?

Done. I also changed `cpus` in lines 846, 986 and 987.


> On March 29, 2016, 12:52 p.m., Joris Van Remoortere wrote:
> > 3rdparty/libprocess/src/process.cpp, line 2208
> > 
> >
> > please use full words for variable names (i.e. `num`)

Changed to `number`.


> On March 29, 2016, 12:52 p.m., Joris Van Remoortere wrote:
> > 3rdparty/libprocess/src/process.cpp, lines 2210-2212
> > 
> >
> > It seems valuable to log the number of threads we will use regardless 
> > of whether they were over-ridden or not.
> > 
> > Let's log the actual number outside of the `if` block. This will also 
> > simplify the message (e.g. `Using X libprocess worker threads`

The number of thread is logged in lines 986 and 987. The added logging here is 
just for the override.
What do you think?


> On March 29, 2016, 12:52 p.m., Joris Van Remoortere wrote:
> > docs/configuration.md, line 1761
> > 
> >
> > Shall we explain what the default is?

Added ", which is the maximum of 8 and the number of cores on the machine".


- Maged


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


On March 29, 2016, 11:14 p.m., Maged Michael wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43144/
> ---
> 
> (Updated March 29, 2016, 11:14 p.m.)
> 
> 
> Review request for mesos, Guangya Liu, Klaus Ma, and Qian Zhang.
> 
> 
> Bugs: MESOS-4353
> https://issues.apache.org/jira/browse/MESOS-4353
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added env var to set upper bound on number of libprocess worker threads.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp feaffa4 
>   docs/configuration.md 9ad0c2a 
> 
> Diff: https://reviews.apache.org/r/43144/diff/
> 
> 
> Testing
> ---
> 
> Passed make check on x86_64 Ubuntu 14.04.
> 
> Invocations of mesos-master, mesos-slave, and mesos-tests with the 
> LIBPROCESS_MAX_WORKER_THREADS set to the values "1", "7", "9", "1000", "0", 
> "-1", and "abc" on a 4-core x86_64 Ubuntu 14.04 system. The results were as 
> expected. 1, 7, 8, 8, 8, 8, and 8 worker threads were created, respectively. 
> The last 3 tests generated warnings.
> 
> 
> Thanks,
> 
> Maged Michael
> 
>

Re: Review Request 43144: Added env var to set upper bound on number of libprocess worker threads.

[Maged Michael]

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

(Updated March 29, 2016, 11:14 p.m.)


Review request for mesos, Guangya Liu, Klaus Ma, and Qian Zhang.


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


Repository: mesos


Description
---

Added env var to set upper bound on number of libprocess worker threads.


Diffs (updated)
-

  3rdparty/libprocess/src/process.cpp feaffa4 
  docs/configuration.md 9ad0c2a 

Diff: https://reviews.apache.org/r/43144/diff/


Testing
---

Passed make check on x86_64 Ubuntu 14.04.

Invocations of mesos-master, mesos-slave, and mesos-tests with the 
LIBPROCESS_MAX_WORKER_THREADS set to the values "1", "7", "9", "1000", "0", 
"-1", and "abc" on a 4-core x86_64 Ubuntu 14.04 system. The results were as 
expected. 1, 7, 8, 8, 8, 8, and 8 worker threads were created, respectively. 
The last 3 tests generated warnings.


Thanks,

Maged Michael

Re: Review Request 43144: Added env var to set upper bound on number of libprocess worker threads.

[Joris Van Remoortere]

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




3rdparty/libprocess/src/process.cpp (line 2193)


since this value will not longer always map to the number of `cpus`, can we 
rename this to something more meaningful such as `num_worker_threads`?



3rdparty/libprocess/src/process.cpp (line 2204)


I don't think we tend to use `auto` for this pattern. Do you mind 
explicitly stating the type?



3rdparty/libprocess/src/process.cpp (line 2208)


please use full words for variable names (i.e. `num`)



3rdparty/libprocess/src/process.cpp (lines 2210 - 2212)


It seems valuable to log the number of threads we will use regardless of 
whether they were over-ridden or not.

Let's log the actual number outside of the `if` block. This will also 
simplify the message (e.g. `Using X libprocess worker threads`



docs/configuration.md (line 1761)


Shall we explain what the default is?


- Joris Van Remoortere


On March 28, 2016, 8:28 p.m., Maged Michael wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43144/
> ---
> 
> (Updated March 28, 2016, 8:28 p.m.)
> 
> 
> Review request for mesos, Guangya Liu, Klaus Ma, and Qian Zhang.
> 
> 
> Bugs: MESOS-4353
> https://issues.apache.org/jira/browse/MESOS-4353
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added env var to set upper bound on number of libprocess worker threads.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp feaffa4 
>   docs/configuration.md 9ad0c2a 
> 
> Diff: https://reviews.apache.org/r/43144/diff/
> 
> 
> Testing
> ---
> 
> Passed make check on x86_64 Ubuntu 14.04.
> 
> Invocations of mesos-master, mesos-slave, and mesos-tests with the 
> LIBPROCESS_MAX_WORKER_THREADS set to the values "1", "7", "9", "1000", "0", 
> "-1", and "abc" on a 4-core x86_64 Ubuntu 14.04 system. The results were as 
> expected. 1, 7, 8, 8, 8, 8, and 8 worker threads were created, respectively. 
> The last 3 tests generated warnings.
> 
> 
> Thanks,
> 
> Maged Michael
> 
>

Re: Review Request 43144: Added env var to set upper bound on number of libprocess worker threads.

[Maged Michael]

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

(Updated March 28, 2016, 8:28 p.m.)


Review request for mesos, Guangya Liu, Klaus Ma, and Qian Zhang.


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


Repository: mesos


Description
---

Added env var to set upper bound on number of libprocess worker threads.


Diffs (updated)
-

  3rdparty/libprocess/src/process.cpp feaffa4 
  docs/configuration.md 9ad0c2a 

Diff: https://reviews.apache.org/r/43144/diff/


Testing
---

Passed make check on x86_64 Ubuntu 14.04.

Invocations of mesos-master, mesos-slave, and mesos-tests with the 
LIBPROCESS_MAX_WORKER_THREADS set to the values "1", "7", "9", "1000", "0", 
"-1", and "abc" on a 4-core x86_64 Ubuntu 14.04 system. The results were as 
expected. 1, 7, 8, 8, 8, 8, and 8 worker threads were created, respectively. 
The last 3 tests generated warnings.


Thanks,

Maged Michael

Re: Review Request 43144: Added env var to set upper bound on number of libprocess worker threads.

[Artem Harutyunyan]

> On Feb. 6, 2016, 7:39 p.m., Guangya Liu wrote:
> > @Maged, I think that you can now find a shephard for this. You can post an 
> > email to d...@mesos.apache.org to ask for a shephard for MESOS-4353

Folks, generally the expectation is that the shepherd is found *before* the 
code gets written and not after the patch has gone through a number of 
interations on the reviewboard.


- Artem


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


On Feb. 6, 2016, 7:34 p.m., Maged Michael wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43144/
> ---
> 
> (Updated Feb. 6, 2016, 7:34 p.m.)
> 
> 
> Review request for mesos, Guangya Liu, Klaus Ma, and Qian Zhang.
> 
> 
> Bugs: MESOS-4353
> https://issues.apache.org/jira/browse/MESOS-4353
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added env var to set upper bound on number of libprocess worker threads.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp d8a74d7 
>   docs/configuration.md 4b5a394 
> 
> Diff: https://reviews.apache.org/r/43144/diff/
> 
> 
> Testing
> ---
> 
> Passed make check on x86_64 Ubuntu 14.04.
> 
> Invocations of mesos-master, mesos-slave, and mesos-tests with the 
> LIBPROCESS_MAX_WORKER_THREADS set to the values "1", "7", "9", "1000", "0", 
> "-1", and "abc" on a 4-core x86_64 Ubuntu 14.04 system. The results were as 
> expected. 1, 7, 8, 8, 8, 8, and 8 worker threads were created, respectively. 
> The last 3 tests generated warnings.
> 
> 
> Thanks,
> 
> Maged Michael
> 
>

Re: Review Request 43144: Added env var to set upper bound on number of libprocess worker threads.

[Maged Michael]

> On Feb. 6, 2016, 10:11 a.m., Guangya Liu wrote:
> > docs/configuration.md, lines 1679-1682
> > 
> >
> > How about: Libprocess will normally use a thread pool sized according 
> > to the number of active CPUs. This can be overridden by setting the 
> > LIBPROCESS_MAX_WORKER_THREADS environment variable to the desired thread 
> > count. Normally, this will provides an upper bound on the number of worker 
> > threads.

The setting is always an upper bound. I tried to follow the same form of the 
descriptions of other environment variables "If set, ...".
How about the following?
  If set, this provides an upper bound on the size of the
  libprocess thread pool. By default, libprocess uses a thread
  pool sized according to the number of CPUs, but not less than 8.


- Maged


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


On Feb. 5, 2016, 9:46 p.m., Maged Michael wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43144/
> ---
> 
> (Updated Feb. 5, 2016, 9:46 p.m.)
> 
> 
> Review request for mesos, Guangya Liu, Klaus Ma, and Qian Zhang.
> 
> 
> Bugs: MESOS-4353
> https://issues.apache.org/jira/browse/MESOS-4353
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added env var to set upper bound on number of libprocess worker threads.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp d8a74d7 
>   docs/configuration.md 4b5a394 
> 
> Diff: https://reviews.apache.org/r/43144/diff/
> 
> 
> Testing
> ---
> 
> Passed make check on x86_64 Ubuntu 14.04.
> 
> Invocations of mesos-master, mesos-slave, and mesos-tests with the 
> LIBPROCESS_MAX_WORKER_THREADS set to the values "1", "7", "9", "1000", "0", 
> "-1", and "abc" on a 4-core x86_64 Ubuntu 14.04 system. The results were as 
> expected. 1, 7, 8, 8, 8, 8, and 8 worker threads were created, respectively. 
> The last 3 tests generated warnings.
> 
> 
> Thanks,
> 
> Maged Michael
> 
>

Re: Review Request 43144: Added env var to set upper bound on number of libprocess worker threads.

[Maged Michael]

> On Feb. 6, 2016, 10:11 a.m., Guangya Liu wrote:
> > docs/configuration.md, lines 1679-1682
> > 
> >
> > How about: Libprocess will normally use a thread pool sized according 
> > to the number of active CPUs. This can be overridden by setting the 
> > LIBPROCESS_MAX_WORKER_THREADS environment variable to the desired thread 
> > count. Normally, this will provides an upper bound on the number of worker 
> > threads.
> 
> Maged Michael wrote:
> The setting is always an upper bound. I tried to follow the same form of 
> the descriptions of other environment variables "If set, ...".
> How about the following?
>   If set, this provides an upper bound on the size of the
>   libprocess thread pool. By default, libprocess uses a thread
>   pool sized according to the number of CPUs, but not less than 8.

How about?
  If set to a positive integer value, this provides an upper bound
  on the size of the libprocess thread pool. By default,
  libprocess uses a thread pool sized according to the number of
  CPUs, but not less than 8.  If set to a valid value, the size of
  the thread pool is min(value, max(8, number of
  CPUs)). Otherwise, it is max(8, number of CPUs).


- Maged


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


On Feb. 5, 2016, 9:46 p.m., Maged Michael wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43144/
> ---
> 
> (Updated Feb. 5, 2016, 9:46 p.m.)
> 
> 
> Review request for mesos, Guangya Liu, Klaus Ma, and Qian Zhang.
> 
> 
> Bugs: MESOS-4353
> https://issues.apache.org/jira/browse/MESOS-4353
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added env var to set upper bound on number of libprocess worker threads.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp d8a74d7 
>   docs/configuration.md 4b5a394 
> 
> Diff: https://reviews.apache.org/r/43144/diff/
> 
> 
> Testing
> ---
> 
> Passed make check on x86_64 Ubuntu 14.04.
> 
> Invocations of mesos-master, mesos-slave, and mesos-tests with the 
> LIBPROCESS_MAX_WORKER_THREADS set to the values "1", "7", "9", "1000", "0", 
> "-1", and "abc" on a 4-core x86_64 Ubuntu 14.04 system. The results were as 
> expected. 1, 7, 8, 8, 8, 8, and 8 worker threads were created, respectively. 
> The last 3 tests generated warnings.
> 
> 
> Thanks,
> 
> Maged Michael
> 
>

Re: Review Request 43144: Added env var to set upper bound on number of libprocess worker threads.

[Guangya Liu]

> On 二月 6, 2016, 10:11 a.m., Guangya Liu wrote:
> > docs/configuration.md, lines 1679-1682
> > 
> >
> > How about: Libprocess will normally use a thread pool sized according 
> > to the number of active CPUs. This can be overridden by setting the 
> > LIBPROCESS_MAX_WORKER_THREADS environment variable to the desired thread 
> > count. Normally, this will provides an upper bound on the number of worker 
> > threads.
> 
> Maged Michael wrote:
> The setting is always an upper bound. I tried to follow the same form of 
> the descriptions of other environment variables "If set, ...".
> How about the following?
>   If set, this provides an upper bound on the size of the
>   libprocess thread pool. By default, libprocess uses a thread
>   pool sized according to the number of CPUs, but not less than 8.
> 
> Maged Michael wrote:
> How about?
>   If set to a positive integer value, this provides an upper bound
>   on the size of the libprocess thread pool. By default,
>   libprocess uses a thread pool sized according to the number of
>   CPUs, but not less than 8.  If set to a valid value, the size of
>   the thread pool is min(value, max(8, number of
>   CPUs)). Otherwise, it is max(8, number of CPUs).

What about the following? I think that we can remove the first sentense.
  This provides an upper bound on the size of the libprocess thread pool. 
By default,
  libprocess uses a thread pool sized according to the number of
  CPUs, but not less than 8.  If set to a valid value, the size of
  the thread pool is min(value, max(8, number of
  CPUs)). Otherwise, it is max(8, number of CPUs).


- Guangya


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


On 二月 5, 2016, 9:46 p.m., Maged Michael wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43144/
> ---
> 
> (Updated 二月 5, 2016, 9:46 p.m.)
> 
> 
> Review request for mesos, Guangya Liu, Klaus Ma, and Qian Zhang.
> 
> 
> Bugs: MESOS-4353
> https://issues.apache.org/jira/browse/MESOS-4353
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added env var to set upper bound on number of libprocess worker threads.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp d8a74d7 
>   docs/configuration.md 4b5a394 
> 
> Diff: https://reviews.apache.org/r/43144/diff/
> 
> 
> Testing
> ---
> 
> Passed make check on x86_64 Ubuntu 14.04.
> 
> Invocations of mesos-master, mesos-slave, and mesos-tests with the 
> LIBPROCESS_MAX_WORKER_THREADS set to the values "1", "7", "9", "1000", "0", 
> "-1", and "abc" on a 4-core x86_64 Ubuntu 14.04 system. The results were as 
> expected. 1, 7, 8, 8, 8, 8, and 8 worker threads were created, respectively. 
> The last 3 tests generated warnings.
> 
> 
> Thanks,
> 
> Maged Michael
> 
>

Re: Review Request 43144: Added env var to set upper bound on number of libprocess worker threads.

[Guangya Liu]

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



@Maged, I think that you can now find a shephard for this. You can post an 
email to d...@mesos.apache.org to ask for a shephard for MESOS-4353

- Guangya Liu


On 二月 7, 2016, 3:34 a.m., Maged Michael wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43144/
> ---
> 
> (Updated 二月 7, 2016, 3:34 a.m.)
> 
> 
> Review request for mesos, Guangya Liu, Klaus Ma, and Qian Zhang.
> 
> 
> Bugs: MESOS-4353
> https://issues.apache.org/jira/browse/MESOS-4353
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added env var to set upper bound on number of libprocess worker threads.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp d8a74d7 
>   docs/configuration.md 4b5a394 
> 
> Diff: https://reviews.apache.org/r/43144/diff/
> 
> 
> Testing
> ---
> 
> Passed make check on x86_64 Ubuntu 14.04.
> 
> Invocations of mesos-master, mesos-slave, and mesos-tests with the 
> LIBPROCESS_MAX_WORKER_THREADS set to the values "1", "7", "9", "1000", "0", 
> "-1", and "abc" on a 4-core x86_64 Ubuntu 14.04 system. The results were as 
> expected. 1, 7, 8, 8, 8, 8, and 8 worker threads were created, respectively. 
> The last 3 tests generated warnings.
> 
> 
> Thanks,
> 
> Maged Michael
> 
>

Re: Review Request 43144: Added env var to set upper bound on number of libprocess worker threads.

[Maged Michael]

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

(Updated Feb. 7, 2016, 3:34 a.m.)


Review request for mesos, Guangya Liu, Klaus Ma, and Qian Zhang.


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


Repository: mesos


Description
---

Added env var to set upper bound on number of libprocess worker threads.


Diffs (updated)
-

  3rdparty/libprocess/src/process.cpp d8a74d7 
  docs/configuration.md 4b5a394 

Diff: https://reviews.apache.org/r/43144/diff/


Testing
---

Passed make check on x86_64 Ubuntu 14.04.

Invocations of mesos-master, mesos-slave, and mesos-tests with the 
LIBPROCESS_MAX_WORKER_THREADS set to the values "1", "7", "9", "1000", "0", 
"-1", and "abc" on a 4-core x86_64 Ubuntu 14.04 system. The results were as 
expected. 1, 7, 8, 8, 8, 8, and 8 worker threads were created, respectively. 
The last 3 tests generated warnings.


Thanks,

Maged Michael

Re: Review Request 43144: Added env var to set upper bound on number of libprocess worker threads.

[Guangya Liu]

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




docs/configuration.md (lines 1679 - 1682)


How about: Libprocess will normally use a thread pool sized according to 
the number of active CPUs. This can be overridden by setting the 
LIBPROCESS_MAX_WORKER_THREADS environment variable to the desired thread count. 
Normally, this will provides an upper bound on the number of worker threads.


- Guangya Liu


On 二月 5, 2016, 9:46 p.m., Maged Michael wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43144/
> ---
> 
> (Updated 二月 5, 2016, 9:46 p.m.)
> 
> 
> Review request for mesos, Guangya Liu, Klaus Ma, and Qian Zhang.
> 
> 
> Bugs: MESOS-4353
> https://issues.apache.org/jira/browse/MESOS-4353
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added env var to set upper bound on number of libprocess worker threads.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp d8a74d7 
>   docs/configuration.md 4b5a394 
> 
> Diff: https://reviews.apache.org/r/43144/diff/
> 
> 
> Testing
> ---
> 
> Passed make check on x86_64 Ubuntu 14.04.
> 
> Invocations of mesos-master, mesos-slave, and mesos-tests with the 
> LIBPROCESS_MAX_WORKER_THREADS set to the values "1", "7", "9", "1000", "0", 
> "-1", and "abc" on a 4-core x86_64 Ubuntu 14.04 system. The results were as 
> expected. 1, 7, 8, 8, 8, 8, and 8 worker threads were created, respectively. 
> The last 3 tests generated warnings.
> 
> 
> Thanks,
> 
> Maged Michael
> 
>

Re: Review Request 43144: Added env var to set upper bound on number of libprocess worker threads.

[Maged Michael]

> On Feb. 5, 2016, 2:20 p.m., Qian Zhang wrote:
> > 1. Besides changing the code, you may also need to update the following doc 
> > to describe the newly introduced env var "LIBPROCESS_MAX_WORKER_THREADS"
> > https://github.com/apache/mesos/blob/master/docs/configuration.md#libprocess-options
> > 2. Have you test the patch with executor from E2E? For example, start Mesos 
> > agent with "LIBPROCESS_MAX_WORKER_THREADS" in its 
> > "--executor_environment_variables" option, launch a task, and check if the 
> > libprocess worker thread number of the related executor is the expected 
> > number.

1. Done.
2. No. Can someone point me to a simple test to view executor logs?


- Maged


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


On Feb. 5, 2016, 9:46 p.m., Maged Michael wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43144/
> ---
> 
> (Updated Feb. 5, 2016, 9:46 p.m.)
> 
> 
> Review request for mesos, Guangya Liu, Klaus Ma, and Qian Zhang.
> 
> 
> Bugs: MESOS-4353
> https://issues.apache.org/jira/browse/MESOS-4353
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added env var to set upper bound on number of libprocess worker threads.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp d8a74d7 
>   docs/configuration.md 4b5a394 
> 
> Diff: https://reviews.apache.org/r/43144/diff/
> 
> 
> Testing
> ---
> 
> Passed make check on x86_64 Ubuntu 14.04.
> 
> Invocations of mesos-master, mesos-slave, and mesos-tests with the 
> LIBPROCESS_MAX_WORKER_THREADS set to the values "1", "7", "9", "1000", "0", 
> "-1", and "abc" on a 4-core x86_64 Ubuntu 14.04 system. The results were as 
> expected. 1, 7, 8, 8, 8, 8, and 8 worker threads were created, respectively. 
> The last 3 tests generated warnings.
> 
> 
> Thanks,
> 
> Maged Michael
> 
>

Re: Review Request 43144: Added env var to set upper bound on number of libprocess worker threads.

[Maged Michael]

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

(Updated Feb. 5, 2016, 9:45 p.m.)


Review request for mesos and Klaus Ma.


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


Repository: mesos


Description
---

Added env var to set upper bound on number of libprocess worker threads.


Diffs (updated)
-

  3rdparty/libprocess/src/process.cpp d8a74d7 
  docs/configuration.md 4b5a394 

Diff: https://reviews.apache.org/r/43144/diff/


Testing
---

Passed make check on x86_64 Ubuntu 14.04.

Invocations of mesos-master, mesos-slave, and mesos-tests with the 
LIBPROCESS_MAX_WORKER_THREADS set to the values "1", "7", "9", "1000", "0", 
"-1", and "abc" on a 4-core x86_64 Ubuntu 14.04 system. The results were as 
expected. 1, 7, 8, 8, 8, 8, and 8 worker threads were created, respectively. 
The last 3 tests generated warnings.


Thanks,

Maged Michael

Re: Review Request 43144: Added env var to set upper bound on number of libprocess worker threads.

[Qian Zhang]

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



1. Besides changing the code, you may also need to update the following doc to 
describe the newly introduced env var "LIBPROCESS_MAX_WORKER_THREADS"
https://github.com/apache/mesos/blob/master/docs/configuration.md#libprocess-options
2. Have you test the patch with executor from E2E? For example, start Mesos 
agent with "LIBPROCESS_MAX_WORKER_THREADS" in its 
"--executor_environment_variables" option, launch a task, and check if the 
libprocess worker thread number of the related executor is the expected number.

- Qian Zhang


On Feb. 5, 2016, 11:39 a.m., Maged Michael wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43144/
> ---
> 
> (Updated Feb. 5, 2016, 11:39 a.m.)
> 
> 
> Review request for mesos and Klaus Ma.
> 
> 
> Bugs: MESOS-4353
> https://issues.apache.org/jira/browse/MESOS-4353
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added env var to set upper bound on number of libprocess worker threads.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp d8a74d7 
> 
> Diff: https://reviews.apache.org/r/43144/diff/
> 
> 
> Testing
> ---
> 
> Passed make check on x86_64 Ubuntu 14.04.
> 
> Invocations of mesos-master, mesos-slave, and mesos-tests with the 
> LIBPROCESS_MAX_WORKER_THREADS set to the values "1", "7", "9", "1000", "0", 
> "-1", and "abc" on a 4-core x86_64 Ubuntu 14.04 system. The results were as 
> expected. 1, 7, 8, 8, 8, 8, and 8 worker threads were created, respectively. 
> The last 3 tests generated warnings.
> 
> 
> Thanks,
> 
> Maged Michael
> 
>

Re: Review Request 43144: Added env var to set upper bound on number of libprocess worker threads.

[Klaus Ma]

> On Feb. 4, 2016, 9:38 p.m., Klaus Ma wrote:
> > 3rdparty/libprocess/src/process.cpp, line 2211
> > 
> >
> > Just `os::getenv("LIB...")` is OK.
> 
> Maged Michael wrote:
> The string is used in two places: getenv and VLOG(WARNING). I thought to 
> avoid mismatch between the two literals.
> How about 
>constexpr auto env_var = "LIBPROCESS_MAX_WORKER_THREADS";
> ?

That's OK to me.


- Klaus


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


On Feb. 5, 2016, 3:55 a.m., Maged Michael wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43144/
> ---
> 
> (Updated Feb. 5, 2016, 3:55 a.m.)
> 
> 
> Review request for mesos and Klaus Ma.
> 
> 
> Bugs: MESOS-4353
> https://issues.apache.org/jira/browse/MESOS-4353
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added env var to set upper bound on number of libprocess worker threads.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp d8a74d7 
> 
> Diff: https://reviews.apache.org/r/43144/diff/
> 
> 
> Testing
> ---
> 
> Passed make check on x86_64 Ubuntu 14.04.
> 
> Invocations of mesos-master, mesos-slave, and mesos-tests with the 
> LIBPROCESS_MAX_WORKER_THREADS set to the values "1", "7", "9", "1000", "0", 
> "-1", and "abc" on a 4-core x86_64 Ubuntu 14.04 system. The results were as 
> expected. 1, 7, 8, 8, 8, 8, and 8 worker threads were created, respectively. 
> The last 3 tests generated warnings.
> 
> 
> Thanks,
> 
> Maged Michael
> 
>

Re: Review Request 43144: Added env var to set upper bound on number of libprocess worker threads.

[Guangya Liu]

> On 二月 5, 2016, 2:51 a.m., Guangya Liu wrote:
> > 3rdparty/libprocess/src/process.cpp, lines 2212-2213
> > 
> >
> > Why not 
> > 
> > Option value = os::getenv("LIBPROCESS_MAX_WORKER_THREADS");

I see what you mean: it will be used in the log message.


- Guangya


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


On 二月 4, 2016, 7:55 p.m., Maged Michael wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43144/
> ---
> 
> (Updated 二月 4, 2016, 7:55 p.m.)
> 
> 
> Review request for mesos and Klaus Ma.
> 
> 
> Bugs: MESOS-4353
> https://issues.apache.org/jira/browse/MESOS-4353
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added env var to set upper bound on number of libprocess worker threads.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp d8a74d7 
> 
> Diff: https://reviews.apache.org/r/43144/diff/
> 
> 
> Testing
> ---
> 
> Passed make check on x86_64 Ubuntu 14.04.
> 
> Invocations of mesos-master, mesos-slave, and mesos-tests with the 
> LIBPROCESS_MAX_WORKER_THREADS set to the values "1", "7", "9", "1000", "0", 
> "-1", and "abc" on a 4-core x86_64 Ubuntu 14.04 system. The results were as 
> expected. 1, 7, 8, 8, 8, 8, and 8 worker threads were created, respectively. 
> The last 3 tests generated warnings.
> 
> 
> Thanks,
> 
> Maged Michael
> 
>

Re: Review Request 43144: Added env var to set upper bound on number of libprocess worker threads.

[Guangya Liu]

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



The document should also be updated `docs/configuration.md` by adding some 
explanation for this parameter.


3rdparty/libprocess/src/process.cpp (lines 2212 - 2213)


Why not 

Option value = os::getenv("LIBPROCESS_MAX_WORKER_THREADS");



3rdparty/libprocess/src/process.cpp (line 2220)


I think we should add some logs here if the `cpus` override the 
`max_cpus.get()`, the end user may be confused for such case: Why my env does 
not take effect?



3rdparty/libprocess/src/process.cpp (line )


I think that warning message should also be enhanced that 

`"Ignoring invalid value " << value.get() << " for 
LIBPROCESS_MAX_WORKER_THREADS, using system defined value " << cpus;`


- Guangya Liu


On 二月 4, 2016, 7:55 p.m., Maged Michael wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43144/
> ---
> 
> (Updated 二月 4, 2016, 7:55 p.m.)
> 
> 
> Review request for mesos and Klaus Ma.
> 
> 
> Bugs: MESOS-4353
> https://issues.apache.org/jira/browse/MESOS-4353
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added env var to set upper bound on number of libprocess worker threads.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp d8a74d7 
> 
> Diff: https://reviews.apache.org/r/43144/diff/
> 
> 
> Testing
> ---
> 
> Passed make check on x86_64 Ubuntu 14.04.
> 
> Invocations of mesos-master, mesos-slave, and mesos-tests with the 
> LIBPROCESS_MAX_WORKER_THREADS set to the values "1", "7", "9", "1000", "0", 
> "-1", and "abc" on a 4-core x86_64 Ubuntu 14.04 system. The results were as 
> expected. 1, 7, 8, 8, 8, 8, and 8 worker threads were created, respectively. 
> The last 3 tests generated warnings.
> 
> 
> Thanks,
> 
> Maged Michael
> 
>

Re: Review Request 43144: Added env var to set upper bound on number of libprocess worker threads.

[Guangya Liu]

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




3rdparty/libprocess/src/process.cpp (line )


Can you please also log the system defined number into log message?


- Guangya Liu


On 二月 5, 2016, 3:39 a.m., Maged Michael wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43144/
> ---
> 
> (Updated 二月 5, 2016, 3:39 a.m.)
> 
> 
> Review request for mesos and Klaus Ma.
> 
> 
> Bugs: MESOS-4353
> https://issues.apache.org/jira/browse/MESOS-4353
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added env var to set upper bound on number of libprocess worker threads.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp d8a74d7 
> 
> Diff: https://reviews.apache.org/r/43144/diff/
> 
> 
> Testing
> ---
> 
> Passed make check on x86_64 Ubuntu 14.04.
> 
> Invocations of mesos-master, mesos-slave, and mesos-tests with the 
> LIBPROCESS_MAX_WORKER_THREADS set to the values "1", "7", "9", "1000", "0", 
> "-1", and "abc" on a 4-core x86_64 Ubuntu 14.04 system. The results were as 
> expected. 1, 7, 8, 8, 8, 8, and 8 worker threads were created, respectively. 
> The last 3 tests generated warnings.
> 
> 
> Thanks,
> 
> Maged Michael
> 
>

Re: Review Request 43144: Added env var to set upper bound on number of libprocess worker threads.

[Maged Michael]

> On Feb. 5, 2016, 2:51 a.m., Guangya Liu wrote:
> > 3rdparty/libprocess/src/process.cpp, lines 2212-2213
> > 
> >
> > Why not 
> > 
> > Option value = os::getenv("LIBPROCESS_MAX_WORKER_THREADS");
> 
> Guangya Liu wrote:
> I see what you mean: it will be used in the log message.

The string is used twice, once as argument of getenv and another time in the 
warning. My concern about repeating the string is that there could be a 
mismatch (e.g., we decide to change the environment variabvle but forget to 
update the warning) that is not caught by the compiler.


> On Feb. 5, 2016, 2:51 a.m., Guangya Liu wrote:
> > 3rdparty/libprocess/src/process.cpp, line 2220
> > 
> >
> > I think we should add some logs here if the `cpus` override the 
> > `max_cpus.get()`, the end user may be confused for such case: Why my env 
> > does not take effect?

How about?
  if (max_cpus.get() < cpus) {
cpus = max_cpus.get();
VLOG(1) << "Overriding the system defined number of worker threads, "
<< "using the value " << cpus;
  } else {
VLOG(1) << "The system defined number of worker threads " << cpus
<< " is unchanged, as it is not greater than " << env_var
<< "=" << max_cpus.get();
  }


> On Feb. 5, 2016, 2:51 a.m., Guangya Liu wrote:
> > 3rdparty/libprocess/src/process.cpp, line 
> > 
> >
> > I think that warning message should also be enhanced that 
> > 
> > `"Ignoring invalid value " << value.get() << " for 
> > LIBPROCESS_MAX_WORKER_THREADS, using system defined value " << cpus;`

Done.


- Maged


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


On Feb. 5, 2016, 3:39 a.m., Maged Michael wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43144/
> ---
> 
> (Updated Feb. 5, 2016, 3:39 a.m.)
> 
> 
> Review request for mesos and Klaus Ma.
> 
> 
> Bugs: MESOS-4353
> https://issues.apache.org/jira/browse/MESOS-4353
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added env var to set upper bound on number of libprocess worker threads.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp d8a74d7 
> 
> Diff: https://reviews.apache.org/r/43144/diff/
> 
> 
> Testing
> ---
> 
> Passed make check on x86_64 Ubuntu 14.04.
> 
> Invocations of mesos-master, mesos-slave, and mesos-tests with the 
> LIBPROCESS_MAX_WORKER_THREADS set to the values "1", "7", "9", "1000", "0", 
> "-1", and "abc" on a 4-core x86_64 Ubuntu 14.04 system. The results were as 
> expected. 1, 7, 8, 8, 8, 8, and 8 worker threads were created, respectively. 
> The last 3 tests generated warnings.
> 
> 
> Thanks,
> 
> Maged Michael
> 
>

Re: Review Request 43144: Added env var to set upper bound on number of libprocess worker threads.

[Maged Michael]

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

(Updated Feb. 5, 2016, 3:39 a.m.)


Review request for mesos and Klaus Ma.


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


Repository: mesos


Description
---

Added env var to set upper bound on number of libprocess worker threads.


Diffs (updated)
-

  3rdparty/libprocess/src/process.cpp d8a74d7 

Diff: https://reviews.apache.org/r/43144/diff/


Testing
---

Passed make check on x86_64 Ubuntu 14.04.

Invocations of mesos-master, mesos-slave, and mesos-tests with the 
LIBPROCESS_MAX_WORKER_THREADS set to the values "1", "7", "9", "1000", "0", 
"-1", and "abc" on a 4-core x86_64 Ubuntu 14.04 system. The results were as 
expected. 1, 7, 8, 8, 8, 8, and 8 worker threads were created, respectively. 
The last 3 tests generated warnings.


Thanks,

Maged Michael

Re: Review Request 43144: Added env var to set upper bound on number of libprocess worker threads.

[Guangya Liu]

> On 二月 5, 2016, 2:51 a.m., Guangya Liu wrote:
> > 3rdparty/libprocess/src/process.cpp, line 
> > 
> >
> > I think that warning message should also be enhanced that 
> > 
> > `"Ignoring invalid value " << value.get() << " for 
> > LIBPROCESS_MAX_WORKER_THREADS, using system defined value " << cpus;`
> 
> Maged Michael wrote:
> Done.

@Maged, you can just click `Fixed` button here to mark that the issue has been 
resolved.


- Guangya


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


On 二月 5, 2016, 3:39 a.m., Maged Michael wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43144/
> ---
> 
> (Updated 二月 5, 2016, 3:39 a.m.)
> 
> 
> Review request for mesos and Klaus Ma.
> 
> 
> Bugs: MESOS-4353
> https://issues.apache.org/jira/browse/MESOS-4353
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added env var to set upper bound on number of libprocess worker threads.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp d8a74d7 
> 
> Diff: https://reviews.apache.org/r/43144/diff/
> 
> 
> Testing
> ---
> 
> Passed make check on x86_64 Ubuntu 14.04.
> 
> Invocations of mesos-master, mesos-slave, and mesos-tests with the 
> LIBPROCESS_MAX_WORKER_THREADS set to the values "1", "7", "9", "1000", "0", 
> "-1", and "abc" on a 4-core x86_64 Ubuntu 14.04 system. The results were as 
> expected. 1, 7, 8, 8, 8, 8, and 8 worker threads were created, respectively. 
> The last 3 tests generated warnings.
> 
> 
> Thanks,
> 
> Maged Michael
> 
>

Re: Review Request 43144: Added env var to set upper bound on number of libprocess worker threads.

[Maged Michael]

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

(Updated Feb. 4, 2016, 7:25 p.m.)


Review request for mesos and Klaus Ma.


Repository: mesos


Description
---

Added env var to set upper bound on number of libprocess worker threads.


Diffs (updated)
-

  3rdparty/libprocess/src/process.cpp d8a74d7 

Diff: https://reviews.apache.org/r/43144/diff/


Testing
---

Passed make check on x86_64 Ubuntu 14.04.

Invocations of mesos-master, mesos-slave, and mesos-tests with the 
LIBPROCESS_MAX_WORKER_THREADS set to the values "1", "7", "9", "1000", "0", 
"-1", and "abc" on a 4-core x86_64 Ubuntu 14.04 system. The results were as 
expected. 1, 7, 8, 8, 8, 8, and 8 worker threads were created, respectively. 
The last 3 tests generated warnings.


Thanks,

Maged Michael

Re: Review Request 43144: Added env var to set upper bound on number of libprocess worker threads.

[Maged Michael]

> On Feb. 4, 2016, 1:38 p.m., Klaus Ma wrote:
> > 3rdparty/libprocess/src/process.cpp, line 2202
> > 
> >
> > blank line before.

Fixed. Updated patch.


> On Feb. 4, 2016, 1:38 p.m., Klaus Ma wrote:
> > 3rdparty/libprocess/src/process.cpp, line 2211
> > 
> >
> > Just `os::getenv("LIB...")` is OK.

The string is used in two places: getenv and VLOG(WARNING). I thought to avoid 
mismatch between the two literals.
How about 
   constexpr auto env_var = "LIBPROCESS_MAX_WORKER_THREADS";
?


- Maged


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


On Feb. 4, 2016, 7:25 p.m., Maged Michael wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43144/
> ---
> 
> (Updated Feb. 4, 2016, 7:25 p.m.)
> 
> 
> Review request for mesos and Klaus Ma.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added env var to set upper bound on number of libprocess worker threads.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp d8a74d7 
> 
> Diff: https://reviews.apache.org/r/43144/diff/
> 
> 
> Testing
> ---
> 
> Passed make check on x86_64 Ubuntu 14.04.
> 
> Invocations of mesos-master, mesos-slave, and mesos-tests with the 
> LIBPROCESS_MAX_WORKER_THREADS set to the values "1", "7", "9", "1000", "0", 
> "-1", and "abc" on a 4-core x86_64 Ubuntu 14.04 system. The results were as 
> expected. 1, 7, 8, 8, 8, 8, and 8 worker threads were created, respectively. 
> The last 3 tests generated warnings.
> 
> 
> Thanks,
> 
> Maged Michael
> 
>

Re: Review Request 43144: Added env var to set upper bound on number of libprocess worker threads.

[Maged Michael]

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

(Updated Feb. 4, 2016, 7:09 p.m.)


Review request for mesos and Klaus Ma.


Summary (updated)
-

Added env var to set upper bound on number of libprocess worker threads.


Repository: mesos


Description (updated)
---

Added env var to set upper bound on number of libprocess worker threads.


Diffs (updated)
-

  3rdparty/libprocess/src/process.cpp d8a74d7637d20c81f384e974e4fdeba22effb437 

Diff: https://reviews.apache.org/r/43144/diff/


Testing (updated)
---

Passed make check on x86_64 Ubuntu 14.04.

Invocations of mesos-master, mesos-slave, and mesos-tests with the 
LIBPROCESS_MAX_WORKER_THREADS set to the values "1", "7", "9", "1000", "0", 
"-1", and "abc" on a 4-core x86_64 Ubuntu 14.04 system. The results were as 
expected. 1, 7, 8, 8, 8, 8, and 8 worker threads were created, respectively. 
The last 3 tests generated warnings.


Thanks,

Maged Michael

Re: Review Request 43144: Added env var to set upper bound on number of libprocess worker threads.

[Maged Michael]

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

(Updated Feb. 4, 2016, 7:55 p.m.)


Review request for mesos and Klaus Ma.


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


Repository: mesos


Description
---

Added env var to set upper bound on number of libprocess worker threads.


Diffs
-

  3rdparty/libprocess/src/process.cpp d8a74d7 

Diff: https://reviews.apache.org/r/43144/diff/


Testing
---

Passed make check on x86_64 Ubuntu 14.04.

Invocations of mesos-master, mesos-slave, and mesos-tests with the 
LIBPROCESS_MAX_WORKER_THREADS set to the values "1", "7", "9", "1000", "0", 
"-1", and "abc" on a 4-core x86_64 Ubuntu 14.04 system. The results were as 
expected. 1, 7, 8, 8, 8, 8, and 8 worker threads were created, respectively. 
The last 3 tests generated warnings.


Thanks,

Maged Michael