You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Alexander Rukletsov <ru...@gmail.com> on 2018/10/11 14:16:40 UTC

Review Request 68994: Logged request processing time for some endpoints.

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

Review request for mesos, Benjamin Hindman, Benno Evers, Benjamin Mahler, Gastón Kleiman, and Till Toenshoff.


Repository: mesos


Description
-------

This patch leverages `logResponse()` function to print response status
code together with the request processing time for some endpoints on
the master and agent. Not all endpoints are participating to avoid
unnecessary log pollution; only these that are know to be "slow" in
generating the response.

Note that requests are still logged when they are fetched from the
actor mailbox, i.e., affected endpoints now log twice per request:
when the processing starts and when the response is ready to be sent.


Diffs
-----

  src/master/master.cpp 06d769aeba16586a020729d454f4d00688b78c78 
  src/slave/slave.cpp 9d84dcb52e5b7e507fc375f184c5f77be08d70a2 


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


Testing
-------

`make check` on Mac OS 10.13.6 and various Linux distros.

Manual testing
==============
Started a mesos-master process locally: `./bin/mesos-master.sh --work_dir=m`

1. `/state` request: `http http://192.168.1.3:5050/state`
Relevant snippet from the master log:
```
I1011 16:07:05.978631 10211328 http.cpp:1178] HTTP GET for /master/state from 192.168.1.3:56510 with User-Agent='HTTPie/0.9.3'
I1011 16:07:05.981534 13430784 http.cpp:1195] HTTP GET for /master/state from 192.168.1.3:56510: '200 OK' after 3.260928ms
```

2. `/flags` request: `http http://192.168.1.3:5050/flags`
Relevant snippet from the master log:
```
I1011 16:08:51.012370 63365120 http.cpp:1178] HTTP GET for /master/flags from 192.168.1.3:56629 with User-Agent='HTTPie/0.9.3'
```


Thanks,

Alexander Rukletsov


Re: Review Request 68994: Logged request processing time for some endpoints.

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



PASS: Mesos patch 68994 was successfully built and tested.

Reviews applied: `['68992', '68993', '68994']`

All the build artifacts available at: http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2473/mesos-review-68994

- Mesos Reviewbot Windows


On Oct. 16, 2018, 11:57 a.m., Alexander Rukletsov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68994/
> -----------------------------------------------------------
> 
> (Updated Oct. 16, 2018, 11:57 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Benno Evers, Benjamin Mahler, Gastón Kleiman, and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch leverages `logResponse()` function to print response status
> code together with the request processing time for some endpoints on
> the master and agent. Not all endpoints are participating to avoid
> unnecessary log pollution; only these that are know to be "slow" in
> generating the response.
> 
> Note that requests are still logged when they are fetched from the
> actor mailbox, i.e., affected endpoints now log twice per request:
> when the processing starts and when the response is ready to be sent.
> 
> 
> Diffs
> -----
> 
>   src/master/master.cpp 06d769aeba16586a020729d454f4d00688b78c78 
>   src/slave/slave.cpp 9d84dcb52e5b7e507fc375f184c5f77be08d70a2 
> 
> 
> Diff: https://reviews.apache.org/r/68994/diff/2/
> 
> 
> Testing
> -------
> 
> `make check` on Mac OS 10.13.6 and various Linux distros.
> 
> Manual testing
> ==============
> Started a mesos-master process locally: `./bin/mesos-master.sh --work_dir=m`
> 
> 1. `/state` request: `http http://192.168.1.3:5050/state`
> Relevant snippet from the master log:
> ```
> I1011 16:07:05.978631 10211328 http.cpp:1178] HTTP GET for /master/state from 192.168.1.3:56510 with User-Agent='HTTPie/0.9.3'
> I1011 16:07:05.981534 13430784 http.cpp:1195] HTTP GET for /master/state from 192.168.1.3:56510: '200 OK' after 3.260928ms
> ```
> 
> 2. `/flags` request: `http http://192.168.1.3:5050/flags`
> Relevant snippet from the master log:
> ```
> I1011 16:08:51.012370 63365120 http.cpp:1178] HTTP GET for /master/flags from 192.168.1.3:56629 with User-Agent='HTTPie/0.9.3'
> ```
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>


Re: Review Request 68994: Logged request processing time for some endpoints.

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

(Updated Oct. 16, 2018, 11:57 a.m.)


Review request for mesos, Benjamin Hindman, Benno Evers, Benjamin Mahler, Gastón Kleiman, and Till Toenshoff.


Repository: mesos


Description
-------

This patch leverages `logResponse()` function to print response status
code together with the request processing time for some endpoints on
the master and agent. Not all endpoints are participating to avoid
unnecessary log pollution; only these that are know to be "slow" in
generating the response.

Note that requests are still logged when they are fetched from the
actor mailbox, i.e., affected endpoints now log twice per request:
when the processing starts and when the response is ready to be sent.


Diffs (updated)
-----

  src/master/master.cpp 06d769aeba16586a020729d454f4d00688b78c78 
  src/slave/slave.cpp 9d84dcb52e5b7e507fc375f184c5f77be08d70a2 


Diff: https://reviews.apache.org/r/68994/diff/2/

Changes: https://reviews.apache.org/r/68994/diff/1-2/


Testing
-------

`make check` on Mac OS 10.13.6 and various Linux distros.

Manual testing
==============
Started a mesos-master process locally: `./bin/mesos-master.sh --work_dir=m`

1. `/state` request: `http http://192.168.1.3:5050/state`
Relevant snippet from the master log:
```
I1011 16:07:05.978631 10211328 http.cpp:1178] HTTP GET for /master/state from 192.168.1.3:56510 with User-Agent='HTTPie/0.9.3'
I1011 16:07:05.981534 13430784 http.cpp:1195] HTTP GET for /master/state from 192.168.1.3:56510: '200 OK' after 3.260928ms
```

2. `/flags` request: `http http://192.168.1.3:5050/flags`
Relevant snippet from the master log:
```
I1011 16:08:51.012370 63365120 http.cpp:1178] HTTP GET for /master/flags from 192.168.1.3:56629 with User-Agent='HTTPie/0.9.3'
```


Thanks,

Alexander Rukletsov


Re: Review Request 68994: Logged request processing time for some endpoints.

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



Patch looks great!

Reviews applied: [68992, 68993, 68994]

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, 2018, 2:16 p.m., Alexander Rukletsov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68994/
> -----------------------------------------------------------
> 
> (Updated Oct. 11, 2018, 2:16 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Benno Evers, Benjamin Mahler, Gastón Kleiman, and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch leverages `logResponse()` function to print response status
> code together with the request processing time for some endpoints on
> the master and agent. Not all endpoints are participating to avoid
> unnecessary log pollution; only these that are know to be "slow" in
> generating the response.
> 
> Note that requests are still logged when they are fetched from the
> actor mailbox, i.e., affected endpoints now log twice per request:
> when the processing starts and when the response is ready to be sent.
> 
> 
> Diffs
> -----
> 
>   src/master/master.cpp 06d769aeba16586a020729d454f4d00688b78c78 
>   src/slave/slave.cpp 9d84dcb52e5b7e507fc375f184c5f77be08d70a2 
> 
> 
> Diff: https://reviews.apache.org/r/68994/diff/1/
> 
> 
> Testing
> -------
> 
> `make check` on Mac OS 10.13.6 and various Linux distros.
> 
> Manual testing
> ==============
> Started a mesos-master process locally: `./bin/mesos-master.sh --work_dir=m`
> 
> 1. `/state` request: `http http://192.168.1.3:5050/state`
> Relevant snippet from the master log:
> ```
> I1011 16:07:05.978631 10211328 http.cpp:1178] HTTP GET for /master/state from 192.168.1.3:56510 with User-Agent='HTTPie/0.9.3'
> I1011 16:07:05.981534 13430784 http.cpp:1195] HTTP GET for /master/state from 192.168.1.3:56510: '200 OK' after 3.260928ms
> ```
> 
> 2. `/flags` request: `http http://192.168.1.3:5050/flags`
> Relevant snippet from the master log:
> ```
> I1011 16:08:51.012370 63365120 http.cpp:1178] HTTP GET for /master/flags from 192.168.1.3:56629 with User-Agent='HTTPie/0.9.3'
> ```
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>


Re: Review Request 68994: Logged request processing time for some endpoints.

Posted by Till Toenshoff via Review Board <no...@reviews.apache.org>.

> On Oct. 12, 2018, 10:32 p.m., Benno Evers wrote:
> > As we already discussed offline, it's a bit unfortunate to have some code duplication here and only have the statistics enabled for selected endpoints. (for those unaware, this is done in order to avoid spamming the logs with request statistics for every requested static html page, javascript file, css template, etc.)
> > 
> > However, with the choices being between committing this, delaying the patch series to implement an alternative approach, or dropping the idea entirely, I still think the trade-offs favor committing this since it will help to solve some real issues for Mesos users, and doesn't add a big maintenance burden for the future. So I'm giving it a 'Ship It' as is.

Excellent reviewer comment!


- Till


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


On Oct. 11, 2018, 2:16 p.m., Alexander Rukletsov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68994/
> -----------------------------------------------------------
> 
> (Updated Oct. 11, 2018, 2:16 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Benno Evers, Benjamin Mahler, Gastón Kleiman, and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch leverages `logResponse()` function to print response status
> code together with the request processing time for some endpoints on
> the master and agent. Not all endpoints are participating to avoid
> unnecessary log pollution; only these that are know to be "slow" in
> generating the response.
> 
> Note that requests are still logged when they are fetched from the
> actor mailbox, i.e., affected endpoints now log twice per request:
> when the processing starts and when the response is ready to be sent.
> 
> 
> Diffs
> -----
> 
>   src/master/master.cpp 06d769aeba16586a020729d454f4d00688b78c78 
>   src/slave/slave.cpp 9d84dcb52e5b7e507fc375f184c5f77be08d70a2 
> 
> 
> Diff: https://reviews.apache.org/r/68994/diff/1/
> 
> 
> Testing
> -------
> 
> `make check` on Mac OS 10.13.6 and various Linux distros.
> 
> Manual testing
> ==============
> Started a mesos-master process locally: `./bin/mesos-master.sh --work_dir=m`
> 
> 1. `/state` request: `http http://192.168.1.3:5050/state`
> Relevant snippet from the master log:
> ```
> I1011 16:07:05.978631 10211328 http.cpp:1178] HTTP GET for /master/state from 192.168.1.3:56510 with User-Agent='HTTPie/0.9.3'
> I1011 16:07:05.981534 13430784 http.cpp:1195] HTTP GET for /master/state from 192.168.1.3:56510: '200 OK' after 3.260928ms
> ```
> 
> 2. `/flags` request: `http http://192.168.1.3:5050/flags`
> Relevant snippet from the master log:
> ```
> I1011 16:08:51.012370 63365120 http.cpp:1178] HTTP GET for /master/flags from 192.168.1.3:56629 with User-Agent='HTTPie/0.9.3'
> ```
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>


Re: Review Request 68994: Logged request processing time for some endpoints.

Posted by Benno Evers <be...@mesosphere.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68994/#review209506
-----------------------------------------------------------


Ship it!




As we already discussed offline, it's a bit unfortunate to have some code duplication here and only have the statistics enabled for selected endpoints. (for those unaware, this is done in order to avoid spamming the logs with request statistics for every requested static html page, javascript file, css template, etc.)

However, with the choices being between committing this, delaying the patch series to implement an alternative approach, or dropping the idea entirely, I still think the trade-offs favor committing this since it will help to solve some real issues for Mesos users, and doesn't add a big maintenance burden for the future. So I'm giving it a 'Ship It' as is.

- Benno Evers


On Oct. 11, 2018, 2:16 p.m., Alexander Rukletsov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68994/
> -----------------------------------------------------------
> 
> (Updated Oct. 11, 2018, 2:16 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Benno Evers, Benjamin Mahler, Gastón Kleiman, and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch leverages `logResponse()` function to print response status
> code together with the request processing time for some endpoints on
> the master and agent. Not all endpoints are participating to avoid
> unnecessary log pollution; only these that are know to be "slow" in
> generating the response.
> 
> Note that requests are still logged when they are fetched from the
> actor mailbox, i.e., affected endpoints now log twice per request:
> when the processing starts and when the response is ready to be sent.
> 
> 
> Diffs
> -----
> 
>   src/master/master.cpp 06d769aeba16586a020729d454f4d00688b78c78 
>   src/slave/slave.cpp 9d84dcb52e5b7e507fc375f184c5f77be08d70a2 
> 
> 
> Diff: https://reviews.apache.org/r/68994/diff/1/
> 
> 
> Testing
> -------
> 
> `make check` on Mac OS 10.13.6 and various Linux distros.
> 
> Manual testing
> ==============
> Started a mesos-master process locally: `./bin/mesos-master.sh --work_dir=m`
> 
> 1. `/state` request: `http http://192.168.1.3:5050/state`
> Relevant snippet from the master log:
> ```
> I1011 16:07:05.978631 10211328 http.cpp:1178] HTTP GET for /master/state from 192.168.1.3:56510 with User-Agent='HTTPie/0.9.3'
> I1011 16:07:05.981534 13430784 http.cpp:1195] HTTP GET for /master/state from 192.168.1.3:56510: '200 OK' after 3.260928ms
> ```
> 
> 2. `/flags` request: `http http://192.168.1.3:5050/flags`
> Relevant snippet from the master log:
> ```
> I1011 16:08:51.012370 63365120 http.cpp:1178] HTTP GET for /master/flags from 192.168.1.3:56629 with User-Agent='HTTPie/0.9.3'
> ```
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>


Re: Review Request 68994: Logged request processing time for some endpoints.

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



PASS: Mesos patch 68994 was successfully built and tested.

Reviews applied: `['68992', '68993', '68994']`

All the build artifacts available at: http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2452/mesos-review-68994

- Mesos Reviewbot Windows


On Oct. 11, 2018, 10:16 p.m., Alexander Rukletsov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68994/
> -----------------------------------------------------------
> 
> (Updated Oct. 11, 2018, 10:16 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Benno Evers, Benjamin Mahler, Gastón Kleiman, and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch leverages `logResponse()` function to print response status
> code together with the request processing time for some endpoints on
> the master and agent. Not all endpoints are participating to avoid
> unnecessary log pollution; only these that are know to be "slow" in
> generating the response.
> 
> Note that requests are still logged when they are fetched from the
> actor mailbox, i.e., affected endpoints now log twice per request:
> when the processing starts and when the response is ready to be sent.
> 
> 
> Diffs
> -----
> 
>   src/master/master.cpp 06d769aeba16586a020729d454f4d00688b78c78 
>   src/slave/slave.cpp 9d84dcb52e5b7e507fc375f184c5f77be08d70a2 
> 
> 
> Diff: https://reviews.apache.org/r/68994/diff/1/
> 
> 
> Testing
> -------
> 
> `make check` on Mac OS 10.13.6 and various Linux distros.
> 
> Manual testing
> ==============
> Started a mesos-master process locally: `./bin/mesos-master.sh --work_dir=m`
> 
> 1. `/state` request: `http http://192.168.1.3:5050/state`
> Relevant snippet from the master log:
> ```
> I1011 16:07:05.978631 10211328 http.cpp:1178] HTTP GET for /master/state from 192.168.1.3:56510 with User-Agent='HTTPie/0.9.3'
> I1011 16:07:05.981534 13430784 http.cpp:1195] HTTP GET for /master/state from 192.168.1.3:56510: '200 OK' after 3.260928ms
> ```
> 
> 2. `/flags` request: `http http://192.168.1.3:5050/flags`
> Relevant snippet from the master log:
> ```
> I1011 16:08:51.012370 63365120 http.cpp:1178] HTTP GET for /master/flags from 192.168.1.3:56629 with User-Agent='HTTPie/0.9.3'
> ```
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>


Re: Review Request 68994: Logged request processing time for some endpoints.

Posted by Till Toenshoff via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68994/#review209611
-----------------------------------------------------------


Ship it!




Ship It!

- Till Toenshoff


On Oct. 11, 2018, 2:16 p.m., Alexander Rukletsov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68994/
> -----------------------------------------------------------
> 
> (Updated Oct. 11, 2018, 2:16 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Benno Evers, Benjamin Mahler, Gastón Kleiman, and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch leverages `logResponse()` function to print response status
> code together with the request processing time for some endpoints on
> the master and agent. Not all endpoints are participating to avoid
> unnecessary log pollution; only these that are know to be "slow" in
> generating the response.
> 
> Note that requests are still logged when they are fetched from the
> actor mailbox, i.e., affected endpoints now log twice per request:
> when the processing starts and when the response is ready to be sent.
> 
> 
> Diffs
> -----
> 
>   src/master/master.cpp 06d769aeba16586a020729d454f4d00688b78c78 
>   src/slave/slave.cpp 9d84dcb52e5b7e507fc375f184c5f77be08d70a2 
> 
> 
> Diff: https://reviews.apache.org/r/68994/diff/1/
> 
> 
> Testing
> -------
> 
> `make check` on Mac OS 10.13.6 and various Linux distros.
> 
> Manual testing
> ==============
> Started a mesos-master process locally: `./bin/mesos-master.sh --work_dir=m`
> 
> 1. `/state` request: `http http://192.168.1.3:5050/state`
> Relevant snippet from the master log:
> ```
> I1011 16:07:05.978631 10211328 http.cpp:1178] HTTP GET for /master/state from 192.168.1.3:56510 with User-Agent='HTTPie/0.9.3'
> I1011 16:07:05.981534 13430784 http.cpp:1195] HTTP GET for /master/state from 192.168.1.3:56510: '200 OK' after 3.260928ms
> ```
> 
> 2. `/flags` request: `http http://192.168.1.3:5050/flags`
> Relevant snippet from the master log:
> ```
> I1011 16:08:51.012370 63365120 http.cpp:1178] HTTP GET for /master/flags from 192.168.1.3:56629 with User-Agent='HTTPie/0.9.3'
> ```
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>