You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Kevin Klues <kl...@gmail.com> on 2016/04/07 03:54:34 UTC

Review Request 45852: Added standard metrics for GPU resources.

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

Review request for mesos, Ben Mahler, Rob Todd, and Vikrama Ditya.


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


Repository: mesos


Description
-------

This commit also includes updates to the webui to show these metrics.


Diffs
-----

  src/master/metrics.cpp 30c091198a8fdd6d6a957a351dc37d3dae7788e4 
  src/slave/metrics.cpp 42c66d7d7176232ccc71f1e040bcae99900f49f8 
  src/tests/master_tests.cpp 8f93fbaf2bfd66bbc144a85c0097f45c55ff3491 
  src/tests/slave_tests.cpp 03bb6da687a1bf11d81619839e6730835e5c4d82 
  src/webui/master/static/framework.html ee42d1ab841a4c42d95512ee60d577b1bbb66bc8 
  src/webui/master/static/frameworks.html 15ff1e9cb6c70df8df47a1b939681abde591e010 
  src/webui/master/static/home.html a691084f4992cda65734f5fee3b2f38349737b83 
  src/webui/master/static/js/controllers.js f92affab41f8418cd7e5ea25561a182a1761fd79 
  src/webui/master/static/offers.html 01213e9582f50072a9c729782271269f72972d28 
  src/webui/master/static/slave.html 4419f7c166e8768040dab7dbc6fb64e1382ad272 
  src/webui/master/static/slave_executor.html 5acb676390fe4ed17369143c5aaaaa13202c0981 
  src/webui/master/static/slave_framework.html 4b2b1562f38f002b4659b4a883249f0469307323 
  src/webui/master/static/slaves.html 0cb125a7d95ccc7770916cbffa052f43e8ea3d2c 

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


Testing
-------

Ran:
```
GTEST_FILTER="SlaveTest.MetricsInMetricsEndpoint:SlaveTest.MetricsInMetricsEndpoint" make -j check
SUCCESS
```

Manually opened the web UI, clicked around to make sure all the GPU metrics now showed up.
Also looked at "Inspect Element" to verify that there were no javascript errors when loading.

Specifically:
```
* The Resources section in the left sidebar of the main page
* The Resources section of the table in the Frameworks tab
* The Resources section of the table in the Slaves tab
* The Resources section of the table in the Offers tab
```


Thanks,

Kevin Klues


Re: Review Request 45852: Added standard metrics for GPU resources.

Posted by haosdent huang <ha...@gmail.com>.

> On April 8, 2016, 2:24 a.m., haosdent huang wrote:
> > Do you forgot to update
> > ```
> > diff --git a/src/common/http.cpp b/src/common/http.cpp
> > index 3748c71..d2f75b0 100644
> > --- a/src/common/http.cpp
> > +++ b/src/common/http.cpp
> > @@ -105,6 +105,7 @@ JSON::Object model(const Resources& resources)
> >  {
> >    JSON::Object object;
> >    object.values["cpus"] = 0;
> > +  object.values["gpu"] = 0;
> >    object.values["mem"] = 0;
> >    object.values["disk"] = 0;
> > ```
> > 
> > I saw some field is empty in webui after apply your patch.
> > 
> > As you know, if `xxx.resources.gpus` is `undefined`, `+=` would get `NaN`.

OK, my bad to forgot apply https://reviews.apache.org/r/45854/


- haosdent


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


On April 7, 2016, 1:54 a.m., Kevin Klues wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45852/
> -----------------------------------------------------------
> 
> (Updated April 7, 2016, 1:54 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Rob Todd, and Vikrama Ditya.
> 
> 
> Bugs: MESOS-4624
>     https://issues.apache.org/jira/browse/MESOS-4624
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This commit also includes updates to the webui to show these metrics.
> 
> 
> Diffs
> -----
> 
>   src/master/metrics.cpp 30c091198a8fdd6d6a957a351dc37d3dae7788e4 
>   src/slave/metrics.cpp 42c66d7d7176232ccc71f1e040bcae99900f49f8 
>   src/tests/master_tests.cpp 8f93fbaf2bfd66bbc144a85c0097f45c55ff3491 
>   src/tests/slave_tests.cpp 03bb6da687a1bf11d81619839e6730835e5c4d82 
>   src/webui/master/static/framework.html ee42d1ab841a4c42d95512ee60d577b1bbb66bc8 
>   src/webui/master/static/frameworks.html 15ff1e9cb6c70df8df47a1b939681abde591e010 
>   src/webui/master/static/home.html a691084f4992cda65734f5fee3b2f38349737b83 
>   src/webui/master/static/js/controllers.js f92affab41f8418cd7e5ea25561a182a1761fd79 
>   src/webui/master/static/offers.html 01213e9582f50072a9c729782271269f72972d28 
>   src/webui/master/static/slave.html 4419f7c166e8768040dab7dbc6fb64e1382ad272 
>   src/webui/master/static/slave_executor.html 5acb676390fe4ed17369143c5aaaaa13202c0981 
>   src/webui/master/static/slave_framework.html 4b2b1562f38f002b4659b4a883249f0469307323 
>   src/webui/master/static/slaves.html 0cb125a7d95ccc7770916cbffa052f43e8ea3d2c 
> 
> Diff: https://reviews.apache.org/r/45852/diff/
> 
> 
> Testing
> -------
> 
> Ran:
> ```
> GTEST_FILTER="SlaveTest.MetricsInMetricsEndpoint:SlaveTest.MetricsInMetricsEndpoint" make -j check
> SUCCESS
> ```
> 
> Manually opened the web UI, clicked around to make sure all the GPU metrics now showed up.
> Also looked at "Inspect Element" to verify that there were no javascript errors when loading.
> 
> Specifically:
> ```
> * The Resources section in the left sidebar of the main page
> * The Resources section of the table in the Frameworks tab
> * The Resources section of the table in the Slaves tab
> * The Resources section of the table in the Offers tab
> ```
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>


Re: Review Request 45852: Added standard metrics for GPU resources.

Posted by haosdent huang <ha...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45852/#review127718
-----------------------------------------------------------



Do you forgot to update
```
diff --git a/src/common/http.cpp b/src/common/http.cpp
index 3748c71..d2f75b0 100644
--- a/src/common/http.cpp
+++ b/src/common/http.cpp
@@ -105,6 +105,7 @@ JSON::Object model(const Resources& resources)
 {
   JSON::Object object;
   object.values["cpus"] = 0;
+  object.values["gpu"] = 0;
   object.values["mem"] = 0;
   object.values["disk"] = 0;
```

I saw some field is empty in webui after apply your patch.

As you know, if `xxx.resources.gpus` is `undefined`, `+=` would get `NaN`.

- haosdent huang


On April 7, 2016, 1:54 a.m., Kevin Klues wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45852/
> -----------------------------------------------------------
> 
> (Updated April 7, 2016, 1:54 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Rob Todd, and Vikrama Ditya.
> 
> 
> Bugs: MESOS-4624
>     https://issues.apache.org/jira/browse/MESOS-4624
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This commit also includes updates to the webui to show these metrics.
> 
> 
> Diffs
> -----
> 
>   src/master/metrics.cpp 30c091198a8fdd6d6a957a351dc37d3dae7788e4 
>   src/slave/metrics.cpp 42c66d7d7176232ccc71f1e040bcae99900f49f8 
>   src/tests/master_tests.cpp 8f93fbaf2bfd66bbc144a85c0097f45c55ff3491 
>   src/tests/slave_tests.cpp 03bb6da687a1bf11d81619839e6730835e5c4d82 
>   src/webui/master/static/framework.html ee42d1ab841a4c42d95512ee60d577b1bbb66bc8 
>   src/webui/master/static/frameworks.html 15ff1e9cb6c70df8df47a1b939681abde591e010 
>   src/webui/master/static/home.html a691084f4992cda65734f5fee3b2f38349737b83 
>   src/webui/master/static/js/controllers.js f92affab41f8418cd7e5ea25561a182a1761fd79 
>   src/webui/master/static/offers.html 01213e9582f50072a9c729782271269f72972d28 
>   src/webui/master/static/slave.html 4419f7c166e8768040dab7dbc6fb64e1382ad272 
>   src/webui/master/static/slave_executor.html 5acb676390fe4ed17369143c5aaaaa13202c0981 
>   src/webui/master/static/slave_framework.html 4b2b1562f38f002b4659b4a883249f0469307323 
>   src/webui/master/static/slaves.html 0cb125a7d95ccc7770916cbffa052f43e8ea3d2c 
> 
> Diff: https://reviews.apache.org/r/45852/diff/
> 
> 
> Testing
> -------
> 
> Ran:
> ```
> GTEST_FILTER="SlaveTest.MetricsInMetricsEndpoint:SlaveTest.MetricsInMetricsEndpoint" make -j check
> SUCCESS
> ```
> 
> Manually opened the web UI, clicked around to make sure all the GPU metrics now showed up.
> Also looked at "Inspect Element" to verify that there were no javascript errors when loading.
> 
> Specifically:
> ```
> * The Resources section in the left sidebar of the main page
> * The Resources section of the table in the Frameworks tab
> * The Resources section of the table in the Slaves tab
> * The Resources section of the table in the Offers tab
> ```
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>


Re: Review Request 45852: Added standard metrics for GPU resources.

Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45852/#review127886
-----------------------------------------------------------


Ship it!




Ship It!

- Ben Mahler


On April 8, 2016, 8:34 p.m., Kevin Klues wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45852/
> -----------------------------------------------------------
> 
> (Updated April 8, 2016, 8:34 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Rob Todd, and Vikrama Ditya.
> 
> 
> Bugs: MESOS-4624
>     https://issues.apache.org/jira/browse/MESOS-4624
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added standard metrics for GPU resources.
> 
> 
> Diffs
> -----
> 
>   src/master/metrics.cpp 30c091198a8fdd6d6a957a351dc37d3dae7788e4 
>   src/slave/metrics.cpp 42c66d7d7176232ccc71f1e040bcae99900f49f8 
>   src/tests/master_tests.cpp 8f93fbaf2bfd66bbc144a85c0097f45c55ff3491 
>   src/tests/slave_tests.cpp 03bb6da687a1bf11d81619839e6730835e5c4d82 
> 
> Diff: https://reviews.apache.org/r/45852/diff/
> 
> 
> Testing
> -------
> 
> Ran:
> ```
> GTEST_FILTER="SlaveTest.MetricsInMetricsEndpoint:SlaveTest.MetricsInMetricsEndpoint" make -j check
> SUCCESS
> ```
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>


Re: Review Request 45852: Added standard metrics for GPU resources.

Posted by Kevin Klues <kl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45852/
-----------------------------------------------------------

(Updated April 8, 2016, 8:34 p.m.)


Review request for mesos, Ben Mahler, Rob Todd, and Vikrama Ditya.


Changes
-------

Split the webui updates into a separate commit.


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


Repository: mesos


Description (updated)
-------

Added standard metrics for GPU resources.


Diffs (updated)
-----

  src/master/metrics.cpp 30c091198a8fdd6d6a957a351dc37d3dae7788e4 
  src/slave/metrics.cpp 42c66d7d7176232ccc71f1e040bcae99900f49f8 
  src/tests/master_tests.cpp 8f93fbaf2bfd66bbc144a85c0097f45c55ff3491 
  src/tests/slave_tests.cpp 03bb6da687a1bf11d81619839e6730835e5c4d82 

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


Testing (updated)
-------

Ran:
```
GTEST_FILTER="SlaveTest.MetricsInMetricsEndpoint:SlaveTest.MetricsInMetricsEndpoint" make -j check
SUCCESS
```


Thanks,

Kevin Klues


Re: Review Request 45852: Added standard metrics for GPU resources.

Posted by haosdent huang <ha...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45852/#review127537
-----------------------------------------------------------



I suggest to split webui changes from this patch.

- haosdent huang


On April 7, 2016, 1:54 a.m., Kevin Klues wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45852/
> -----------------------------------------------------------
> 
> (Updated April 7, 2016, 1:54 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Rob Todd, and Vikrama Ditya.
> 
> 
> Bugs: MESOS-4624
>     https://issues.apache.org/jira/browse/MESOS-4624
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This commit also includes updates to the webui to show these metrics.
> 
> 
> Diffs
> -----
> 
>   src/master/metrics.cpp 30c091198a8fdd6d6a957a351dc37d3dae7788e4 
>   src/slave/metrics.cpp 42c66d7d7176232ccc71f1e040bcae99900f49f8 
>   src/tests/master_tests.cpp 8f93fbaf2bfd66bbc144a85c0097f45c55ff3491 
>   src/tests/slave_tests.cpp 03bb6da687a1bf11d81619839e6730835e5c4d82 
>   src/webui/master/static/framework.html ee42d1ab841a4c42d95512ee60d577b1bbb66bc8 
>   src/webui/master/static/frameworks.html 15ff1e9cb6c70df8df47a1b939681abde591e010 
>   src/webui/master/static/home.html a691084f4992cda65734f5fee3b2f38349737b83 
>   src/webui/master/static/js/controllers.js f92affab41f8418cd7e5ea25561a182a1761fd79 
>   src/webui/master/static/offers.html 01213e9582f50072a9c729782271269f72972d28 
>   src/webui/master/static/slave.html 4419f7c166e8768040dab7dbc6fb64e1382ad272 
>   src/webui/master/static/slave_executor.html 5acb676390fe4ed17369143c5aaaaa13202c0981 
>   src/webui/master/static/slave_framework.html 4b2b1562f38f002b4659b4a883249f0469307323 
>   src/webui/master/static/slaves.html 0cb125a7d95ccc7770916cbffa052f43e8ea3d2c 
> 
> Diff: https://reviews.apache.org/r/45852/diff/
> 
> 
> Testing
> -------
> 
> Ran:
> ```
> GTEST_FILTER="SlaveTest.MetricsInMetricsEndpoint:SlaveTest.MetricsInMetricsEndpoint" make -j check
> SUCCESS
> ```
> 
> Manually opened the web UI, clicked around to make sure all the GPU metrics now showed up.
> Also looked at "Inspect Element" to verify that there were no javascript errors when loading.
> 
> Specifically:
> ```
> * The Resources section in the left sidebar of the main page
> * The Resources section of the table in the Frameworks tab
> * The Resources section of the table in the Slaves tab
> * The Resources section of the table in the Offers tab
> ```
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>