You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Gilbert Song <so...@gmail.com> on 2018/05/11 07:40:36 UTC

Re: Review Request 66823: Added per framework metrics for framework events.

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

(Updated May 11, 2018, 12:40 a.m.)


Review request for mesos, Benjamin Mahler, Gaston Kleiman, Greg Mann, Jie Yu, and Vinod Kone.


Repository: mesos


Description
-------

Added per framework metrics for framework events.


Diffs (updated)
-----

  src/master/master.hpp 5ec764b5c7f96bab786084cccf20fd8a17319718 
  src/master/metrics.hpp ec76dbcd1d1fa5349d62ce73fb9603e1986a776b 
  src/master/metrics.cpp e46ead79f3f29e285426f9d061337077f453aa45 


Diff: https://reviews.apache.org/r/66823/diff/4/

Changes: https://reviews.apache.org/r/66823/diff/3-4/


Testing
-------


Thanks,

Gilbert Song

Re: Review Request 66823: Added per framework metrics for framework events.

Posted by Gilbert Song <so...@gmail.com>.

> On May 15, 2018, 2:58 p.m., Greg Mann wrote:
> > src/master/master.hpp
> > Lines 2381 (patched)
> > <https://reviews.apache.org/r/66823/diff/5/?file=2019857#file2019857line2381>
> >
> >     I'm curious what the performance impact of this will be during a period of high event throughput. Do you think it's worth adding the new helper for message -> event conversion now?

Sure we could do that. So the complication in devolve could be waived. but there are a lot of helpers to be added (like what we have in evolve.cpp to handle all messages). that is the reason why I leave a TODO here.


> On May 15, 2018, 2:58 p.m., Greg Mann wrote:
> > src/master/metrics.cpp
> > Lines 612 (patched)
> > <https://reviews.apache.org/r/66823/diff/5/?file=2019859#file2019859line612>
> >
> >     Do we need this temporary variable? Here and below.

yes, unordered_map[]++ would cause compilation problems.


- Gilbert


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


On May 11, 2018, 1:44 a.m., Gilbert Song wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66823/
> -----------------------------------------------------------
> 
> (Updated May 11, 2018, 1:44 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Gaston Kleiman, Greg Mann, Jie Yu, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added per framework metrics for framework events.
> 
> 
> Diffs
> -----
> 
>   src/master/master.hpp 5ec764b5c7f96bab786084cccf20fd8a17319718 
>   src/master/metrics.hpp ec76dbcd1d1fa5349d62ce73fb9603e1986a776b 
>   src/master/metrics.cpp e46ead79f3f29e285426f9d061337077f453aa45 
> 
> 
> Diff: https://reviews.apache.org/r/66823/diff/5/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 66823: Added per framework metrics for framework events.

Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66823/#review203166
-----------------------------------------------------------




src/master/master.hpp
Lines 2381 (patched)
<https://reviews.apache.org/r/66823/#comment285249>

    I'm curious what the performance impact of this will be during a period of high event throughput. Do you think it's worth adding the new helper for message -> event conversion now?



src/master/metrics.hpp
Lines 240 (patched)
<https://reviews.apache.org/r/66823/#comment285259>

    I think another name might be more intuitive for this variable - maybe `event_update_types` or `task_status_update_types` or similar?



src/master/metrics.cpp
Lines 612 (patched)
<https://reviews.apache.org/r/66823/#comment285260>

    Do we need this temporary variable? Here and below.


- Greg Mann


On May 11, 2018, 8:44 a.m., Gilbert Song wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66823/
> -----------------------------------------------------------
> 
> (Updated May 11, 2018, 8:44 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Gaston Kleiman, Greg Mann, Jie Yu, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added per framework metrics for framework events.
> 
> 
> Diffs
> -----
> 
>   src/master/master.hpp 5ec764b5c7f96bab786084cccf20fd8a17319718 
>   src/master/metrics.hpp ec76dbcd1d1fa5349d62ce73fb9603e1986a776b 
>   src/master/metrics.cpp e46ead79f3f29e285426f9d061337077f453aa45 
> 
> 
> Diff: https://reviews.apache.org/r/66823/diff/5/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 66823: Added per framework metrics for framework events.

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



Bad patch!

Reviews applied: [66823]

Failed command: python support/apply-reviews.py -n -r 66823

Error:
The support scripts will be upgraded to Python 3 by July 1st.
Make sure to install Python 3.6 on your machine before.
2018-07-07 02:09:57 URL:https://reviews.apache.org/r/66823/diff/raw/ [3535/3535] -> "66823.patch" [1]
error: patch failed: src/master/metrics.hpp:224
error: src/master/metrics.hpp: patch does not apply
error: patch failed: src/master/metrics.cpp:551
error: src/master/metrics.cpp: patch does not apply

Full log: https://builds.apache.org/job/Mesos-Reviewbot/22869/console

- Mesos Reviewbot


On July 6, 2018, 6:02 p.m., Gilbert Song wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66823/
> -----------------------------------------------------------
> 
> (Updated July 6, 2018, 6:02 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Gastón Kleiman, Greg Mann, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-8844
>     https://issues.apache.org/jira/browse/MESOS-8844
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added per framework metrics for framework events.
> 
> 
> Diffs
> -----
> 
>   src/master/master.hpp 5ec764b5c7f96bab786084cccf20fd8a17319718 
>   src/master/metrics.hpp ec76dbcd1d1fa5349d62ce73fb9603e1986a776b 
>   src/master/metrics.cpp e46ead79f3f29e285426f9d061337077f453aa45 
> 
> 
> Diff: https://reviews.apache.org/r/66823/diff/6/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 66823: Added per framework metrics for framework events.

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



FAIL: Failed to apply the current review.

Failed command: `python.exe .\support\python3\apply-reviews.py -n -r 66823`

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

Relevant logs:

- [apply-review-66823-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66823/logs/apply-review-66823-stdout.log):

```
error: patch failed: src/master/metrics.hpp:224
error: src/master/metrics.hpp: patch does not apply
error: patch failed: src/master/metrics.cpp:551
error: src/master/metrics.cpp: patch does not apply
```

- Mesos Reviewbot Windows


On July 6, 2018, 11:02 a.m., Gilbert Song wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66823/
> -----------------------------------------------------------
> 
> (Updated July 6, 2018, 11:02 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Gastón Kleiman, Greg Mann, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-8844
>     https://issues.apache.org/jira/browse/MESOS-8844
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added per framework metrics for framework events.
> 
> 
> Diffs
> -----
> 
>   src/master/master.hpp 5ec764b5c7f96bab786084cccf20fd8a17319718 
>   src/master/metrics.hpp ec76dbcd1d1fa5349d62ce73fb9603e1986a776b 
>   src/master/metrics.cpp e46ead79f3f29e285426f9d061337077f453aa45 
> 
> 
> Diff: https://reviews.apache.org/r/66823/diff/6/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 66823: Added per framework metrics for framework events.

Posted by Gilbert Song <so...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66823/
-----------------------------------------------------------

(Updated July 6, 2018, 6:02 p.m.)


Review request for mesos, Benjamin Mahler, Gastón Kleiman, Greg Mann, Jie Yu, and Vinod Kone.


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


Repository: mesos


Description
-------

Added per framework metrics for framework events.


Diffs
-----

  src/master/master.hpp 5ec764b5c7f96bab786084cccf20fd8a17319718 
  src/master/metrics.hpp ec76dbcd1d1fa5349d62ce73fb9603e1986a776b 
  src/master/metrics.cpp e46ead79f3f29e285426f9d061337077f453aa45 


Diff: https://reviews.apache.org/r/66823/diff/6/


Testing
-------


Thanks,

Gilbert Song

Re: Review Request 66823: Added per framework metrics for framework events.

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



Bad patch!

Reviews applied: [66823, 67776, 66820, 66819, 66882]

Failed command: python support/apply-reviews.py -n -r 66823

Error:
The support scripts will be upgraded to Python 3 by July 1st.
Make sure to install Python 3.6 on your machine before.
2018-06-29 16:04:28 URL:https://reviews.apache.org/r/66823/diff/raw/ [3535/3535] -> "66823.patch" [1]
error: patch failed: src/master/metrics.cpp:551
error: src/master/metrics.cpp: patch does not apply

Full log: https://builds.apache.org/job/Mesos-Reviewbot/22836/console

- Mesos Reviewbot


On June 28, 2018, 10:41 p.m., Gilbert Song wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66823/
> -----------------------------------------------------------
> 
> (Updated June 28, 2018, 10:41 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Gastón Kleiman, Greg Mann, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-8844
>     https://issues.apache.org/jira/browse/MESOS-8844
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added per framework metrics for framework events.
> 
> 
> Diffs
> -----
> 
>   src/master/master.hpp 5ec764b5c7f96bab786084cccf20fd8a17319718 
>   src/master/metrics.hpp ec76dbcd1d1fa5349d62ce73fb9603e1986a776b 
>   src/master/metrics.cpp e46ead79f3f29e285426f9d061337077f453aa45 
> 
> 
> Diff: https://reviews.apache.org/r/66823/diff/6/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 66823: Added per framework metrics for framework events.

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



FAIL: Failed to apply the current review.

Failed command: `python.exe .\support\python3\apply-reviews.py -n -r 66823`

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

Relevant logs:

- [apply-review-66823-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66823/logs/apply-review-66823-stdout.log):

```
error: patch failed: src/master/metrics.cpp:551
error: src/master/metrics.cpp: patch does not apply
```

- Mesos Reviewbot Windows


On June 28, 2018, 10:41 p.m., Gilbert Song wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66823/
> -----------------------------------------------------------
> 
> (Updated June 28, 2018, 10:41 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Gastón Kleiman, Greg Mann, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-8844
>     https://issues.apache.org/jira/browse/MESOS-8844
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added per framework metrics for framework events.
> 
> 
> Diffs
> -----
> 
>   src/master/master.hpp 5ec764b5c7f96bab786084cccf20fd8a17319718 
>   src/master/metrics.hpp ec76dbcd1d1fa5349d62ce73fb9603e1986a776b 
>   src/master/metrics.cpp e46ead79f3f29e285426f9d061337077f453aa45 
> 
> 
> Diff: https://reviews.apache.org/r/66823/diff/6/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 66823: Added per framework metrics for framework events.

Posted by Gilbert Song <so...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66823/
-----------------------------------------------------------

(Updated June 28, 2018, 10:41 p.m.)


Review request for mesos, Benjamin Mahler, Gastón Kleiman, Greg Mann, Jie Yu, and Vinod Kone.


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


Repository: mesos


Description
-------

Added per framework metrics for framework events.


Diffs
-----

  src/master/master.hpp 5ec764b5c7f96bab786084cccf20fd8a17319718 
  src/master/metrics.hpp ec76dbcd1d1fa5349d62ce73fb9603e1986a776b 
  src/master/metrics.cpp e46ead79f3f29e285426f9d061337077f453aa45 


Diff: https://reviews.apache.org/r/66823/diff/6/


Testing
-------


Thanks,

Gilbert Song

Re: Review Request 66823: Added per framework metrics for framework events.

Posted by Gilbert Song <so...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66823/
-----------------------------------------------------------

(Updated May 15, 2018, 5:41 p.m.)


Review request for mesos, Benjamin Mahler, Gaston Kleiman, Greg Mann, Jie Yu, and Vinod Kone.


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


Repository: mesos


Description
-------

Added per framework metrics for framework events.


Diffs (updated)
-----

  src/master/master.hpp 5ec764b5c7f96bab786084cccf20fd8a17319718 
  src/master/metrics.hpp ec76dbcd1d1fa5349d62ce73fb9603e1986a776b 
  src/master/metrics.cpp e46ead79f3f29e285426f9d061337077f453aa45 


Diff: https://reviews.apache.org/r/66823/diff/6/

Changes: https://reviews.apache.org/r/66823/diff/5-6/


Testing
-------


Thanks,

Gilbert Song

Re: Review Request 66823: Added per framework metrics for framework events.

Posted by Gilbert Song <so...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66823/
-----------------------------------------------------------

(Updated May 11, 2018, 1:44 a.m.)


Review request for mesos, Benjamin Mahler, Gaston Kleiman, Greg Mann, Jie Yu, and Vinod Kone.


Repository: mesos


Description
-------

Added per framework metrics for framework events.


Diffs (updated)
-----

  src/master/master.hpp 5ec764b5c7f96bab786084cccf20fd8a17319718 
  src/master/metrics.hpp ec76dbcd1d1fa5349d62ce73fb9603e1986a776b 
  src/master/metrics.cpp e46ead79f3f29e285426f9d061337077f453aa45 


Diff: https://reviews.apache.org/r/66823/diff/5/

Changes: https://reviews.apache.org/r/66823/diff/4-5/


Testing
-------


Thanks,

Gilbert Song