You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Armand Grillet <ag...@mesosphere.io> on 2017/10/11 13:57:45 UTC

Review Request 62891: Increased level for libprocess verbose logs regarding resumed processes.

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

Review request for mesos, Alexander Rukletsov, Benjamin Mahler, and Till Toenshoff.


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


Repository: mesos


Description
-------

To reduce noisiness, libprocess recurring events
"Resuming <process>" are now visible at level 3 instead of 2.


Diffs
-----

  3rdparty/libprocess/src/process.cpp a81ef747957db5bf388fac1a853c12fdd60d0cf7 


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


Testing
-------

Run `GLOG_v=1 ./mesos-local.sh --log_dir=/tmp/test-mesos/log/ --num_slaves=3 --port=5037 --work_dir=/tmp/test-mesos/work/` and `GLOG_v=2 ./mesos-local.sh --log_dir=/tmp/test-mesos/log/ --num_slaves=3 --port=5037 --work_dir=/tmp/test-mesos/work/` and checked that the new output was as expected.


Thanks,

Armand Grillet


Re: Review Request 62891: Increased level of verbose logs for libprocess actor state transitions.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62891/#review188314
-----------------------------------------------------------



PASS: Mesos patch 62891 was successfully built and tested.

Reviews applied: `['62887', '62891']`

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

- Mesos Reviewbot Windows


On Oct. 11, 2017, 9:57 a.m., Armand Grillet wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62891/
> -----------------------------------------------------------
> 
> (Updated Oct. 11, 2017, 9:57 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, and Till Toenshoff.
> 
> 
> Bugs: MESOS-8074
>     https://issues.apache.org/jira/browse/MESOS-8074
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Increases vlog level of some recurring events to improve readability.
> Specifically, we now log the following events on vlog level three
> instead of level two:
>   "Spawned process <process>",
>   "Resuming <process>",
>   "Cleaning up <process>",
>   "Donating thread to <process> while waiting".
> 
> This does not imply a general change or a holistic approach concerning
> the libprocess verbose logs.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/src/process.cpp a81ef747957db5bf388fac1a853c12fdd60d0cf7 
> 
> 
> Diff: https://reviews.apache.org/r/62891/diff/3/
> 
> 
> Testing
> -------
> 
> Run `GLOG_v=2 ./mesos-local.sh --log_dir=/tmp/test-mesos/log/ --num_slaves=3 --port=5037 --work_dir=/tmp/test-mesos/work/` and `GLOG_v=3 ./mesos-local.sh --log_dir=/tmp/test-mesos/log/ --num_slaves=3 --port=5037 --work_dir=/tmp/test-mesos/work/` and checked that the new output was as expected.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>


Re: Review Request 62891: Increased level of verbose logs for libprocess actor state transitions.

Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62891/#review188315
-----------------------------------------------------------



Patch looks great!

Reviews applied: [62891]

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 Oct. 11, 2017, 1:57 p.m., Armand Grillet wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62891/
> -----------------------------------------------------------
> 
> (Updated Oct. 11, 2017, 1:57 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, and Till Toenshoff.
> 
> 
> Bugs: MESOS-8074
>     https://issues.apache.org/jira/browse/MESOS-8074
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Increases vlog level of some recurring events to improve readability.
> Specifically, we now log the following events on vlog level three
> instead of level two:
>   "Spawned process <process>",
>   "Resuming <process>",
>   "Cleaning up <process>",
>   "Donating thread to <process> while waiting".
> 
> This does not imply a general change or a holistic approach concerning
> the libprocess verbose logs.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/src/process.cpp a81ef747957db5bf388fac1a853c12fdd60d0cf7 
> 
> 
> Diff: https://reviews.apache.org/r/62891/diff/3/
> 
> 
> Testing
> -------
> 
> Run `GLOG_v=2 ./mesos-local.sh --log_dir=/tmp/test-mesos/log/ --num_slaves=3 --port=5037 --work_dir=/tmp/test-mesos/work/` and `GLOG_v=3 ./mesos-local.sh --log_dir=/tmp/test-mesos/log/ --num_slaves=3 --port=5037 --work_dir=/tmp/test-mesos/work/` and checked that the new output was as expected.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>


Re: Review Request 62891: Increased level of verbose logs for libprocess actor state transitions.

Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62891/#review188363
-----------------------------------------------------------


Ship it!




Seems fine to me, I’m more interested in the overall approach. Just to give you some context here. Originally we thought that libprocess shouldn’t log because it’s a library (e.g. glibc doesn’t log, stout returns errors to the user rather than logging), but this doesn’t really work since libprocess is its own runtime.

We thought a good strategy was to map to verbose levels:

```
VLOG(1) -> LOG(ERROR)
VLOG(2) -> LOG(WARNING)
VLOG(3) -> LOG(INFO)
VLOG(4) -> VLOG(1)
...
```

So the user could get errors by turning on vlog 1, warnings by going up to vlog 2, etc.

In hindsight this wasn’t a good idea, as I said in the email on the dev list. We actually want the errors/warnings from libprocess in mesos, without having to turn on vlog1/2 globally. So, when you sent the email, I looked around at how other libraries handle this problem. That’s when I realized that it made most sense to let the user control the logging behavior by passing in the loggers (in our case this is just an interface from glog, since we don’t need to support other logging libraries for now).

- Benjamin Mahler


On Oct. 11, 2017, 1:57 p.m., Armand Grillet wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62891/
> -----------------------------------------------------------
> 
> (Updated Oct. 11, 2017, 1:57 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, and Till Toenshoff.
> 
> 
> Bugs: MESOS-8074
>     https://issues.apache.org/jira/browse/MESOS-8074
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Increases vlog level of some recurring events to improve readability.
> Specifically, we now log the following events on vlog level three
> instead of level two:
>   "Spawned process <process>",
>   "Resuming <process>",
>   "Cleaning up <process>",
>   "Donating thread to <process> while waiting".
> 
> This does not imply a general change or a holistic approach concerning
> the libprocess verbose logs.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/src/process.cpp a81ef747957db5bf388fac1a853c12fdd60d0cf7 
> 
> 
> Diff: https://reviews.apache.org/r/62891/diff/3/
> 
> 
> Testing
> -------
> 
> Run `GLOG_v=2 ./mesos-local.sh --log_dir=/tmp/test-mesos/log/ --num_slaves=3 --port=5037 --work_dir=/tmp/test-mesos/work/` and `GLOG_v=3 ./mesos-local.sh --log_dir=/tmp/test-mesos/log/ --num_slaves=3 --port=5037 --work_dir=/tmp/test-mesos/work/` and checked that the new output was as expected.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>


Re: Review Request 62891: Increased level of one verbose log from libprocess.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62891/#review187710
-----------------------------------------------------------



PASS: Mesos patch 62891 was successfully built and tested.

Reviews applied: `['62891']`

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

- Mesos Reviewbot Windows


On Oct. 11, 2017, 1:57 p.m., Armand Grillet wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62891/
> -----------------------------------------------------------
> 
> (Updated Oct. 11, 2017, 1:57 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, and Till Toenshoff.
> 
> 
> Bugs: MESOS-8074
>     https://issues.apache.org/jira/browse/MESOS-8074
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Increases vlog level regarding libprocess actor state transitions. This
> is a recurring event that we now log on vlog level three instead of
> level two, it does not imply a general change or a holistic approach.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/src/process.cpp a81ef747957db5bf388fac1a853c12fdd60d0cf7 
> 
> 
> Diff: https://reviews.apache.org/r/62891/diff/2/
> 
> 
> Testing
> -------
> 
> Run `GLOG_v=2 ./mesos-local.sh --log_dir=/tmp/test-mesos/log/ --num_slaves=3 --port=5037 --work_dir=/tmp/test-mesos/work/` and `GLOG_v=3 ./mesos-local.sh --log_dir=/tmp/test-mesos/log/ --num_slaves=3 --port=5037 --work_dir=/tmp/test-mesos/work/` and checked that the new output was as expected.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>


Re: Review Request 62891: Increased level of verbose logs for libprocess actor state transitions.

Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62891/#review188533
-----------------------------------------------------------


Ship it!




Ship It!

- Alexander Rukletsov


On Oct. 11, 2017, 1:57 p.m., Armand Grillet wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62891/
> -----------------------------------------------------------
> 
> (Updated Oct. 11, 2017, 1:57 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, and Till Toenshoff.
> 
> 
> Bugs: MESOS-8074
>     https://issues.apache.org/jira/browse/MESOS-8074
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Increases vlog level of some recurring events to improve readability.
> Specifically, we now log the following events on vlog level three
> instead of level two:
>   "Spawned process <process>",
>   "Resuming <process>",
>   "Cleaning up <process>",
>   "Donating thread to <process> while waiting".
> 
> This does not imply a general change or a holistic approach concerning
> the libprocess verbose logs.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/src/process.cpp a81ef747957db5bf388fac1a853c12fdd60d0cf7 
> 
> 
> Diff: https://reviews.apache.org/r/62891/diff/3/
> 
> 
> Testing
> -------
> 
> Run `GLOG_v=2 ./mesos-local.sh --log_dir=/tmp/test-mesos/log/ --num_slaves=3 --port=5037 --work_dir=/tmp/test-mesos/work/` and `GLOG_v=3 ./mesos-local.sh --log_dir=/tmp/test-mesos/log/ --num_slaves=3 --port=5037 --work_dir=/tmp/test-mesos/work/` and checked that the new output was as expected.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>