You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by GitBox <gi...@apache.org> on 2020/08/07 16:47:30 UTC

[GitHub] [beam] mxm opened a new pull request #12499: [BEAM-10602] Fix load test metrics in Grafana dashboard

mxm opened a new pull request #12499:
URL: https://github.com/apache/beam/pull/12499


   This reverts the recent changes to the dashboards and adds a commit which adds a latency and checkpoint duration panel.
   
   Also, it modifies the Flink streaming tests to write into the `_pardo_1` table. This way, the results will show up in the dashboard together with all the other Runners' data.
   
   Post-Commit Tests Status (on master branch)
   ------------------------------------------------------------------------------------------------
   
   Lang | SDK | Dataflow | Flink | Samza | Spark | Twister2
   --- | --- | --- | --- | --- | --- | ---
   Go | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/) | --- | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/) | --- | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/) | ---
   Java | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/) | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/) | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/badge/i
 con)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/) | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/) | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/)<br>[![Build Status](htt
 ps://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/) | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Twister2/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Twister2/lastCompletedBuild/)
   Python | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Python2/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Python2/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Python35/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Python35/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Python38/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Python38/lastCompletedBuild/) | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_
 Py_VR_Dataflow/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow_V2/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow_V2/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Py_ValCont/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Py_ValCont/lastCompletedBuild/) | [![Build Status](https://ci-beam.apache.org/job/beam_PreCommit_Python2_PVR_Flink_Cron/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PreCommit_Python2_PVR_Flink_Cron/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Python35_VR_Flink/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Python35_VR_Flink/lastCompletedBuild/) | --- | [![Build Status](https://ci-beam.apache.org/job/beam_P
 ostCommit_Python_VR_Spark/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Python_VR_Spark/lastCompletedBuild/) | ---
   XLang | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Direct/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Direct/lastCompletedBuild/) | --- | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Flink/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Flink/lastCompletedBuild/) | --- | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Spark/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Spark/lastCompletedBuild/) | ---
   
   Pre-Commit Tests Status (on master branch)
   ------------------------------------------------------------------------------------------------
   
   --- |Java | Python | Go | Website
   --- | --- | --- | --- | ---
   Non-portable | [![Build Status](https://ci-beam.apache.org/job/beam_PreCommit_Java_Cron/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PreCommit_Java_Cron/lastCompletedBuild/) | [![Build Status](https://ci-beam.apache.org/job/beam_PreCommit_Python_Cron/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PreCommit_Python_Cron/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PreCommit_PythonLint_Cron/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PreCommit_PythonLint_Cron/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PreCommit_PythonDocker_Cron/badge/icon)](https://ci-beam.apache.org/job/beam_PreCommit_PythonDocker_Cron/lastCompletedBuild/) | [![Build Status](https://ci-beam.apache.org/job/beam_PreCommit_Go_Cron/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PreCommit_Go_Cron/lastCompletedBuild/) | [![Build Status](https://ci-beam.apache.org/job/b
 eam_PreCommit_Website_Cron/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PreCommit_Website_Cron/lastCompletedBuild/)
   Portable | --- | [![Build Status](https://ci-beam.apache.org/job/beam_PreCommit_Portable_Python_Cron/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PreCommit_Portable_Python_Cron/lastCompletedBuild/) | --- | ---
   
   See [.test-infra/jenkins/README](https://github.com/apache/beam/blob/master/.test-infra/jenkins/README.md) for trigger phrase, status and link of all Jenkins jobs.
   
   
   GitHub Actions Tests Status (on master branch)
   ------------------------------------------------------------------------------------------------
   ![Build python source distribution and wheels](https://github.com/apache/beam/workflows/Build%20python%20source%20distribution%20and%20wheels/badge.svg)
   
   See [CI.md](https://github.com/apache/beam/blob/master/CI.md) for more information about GitHub Actions CI.
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] tysonjh commented on a change in pull request #12499: [BEAM-10602] Fix load test metrics in Grafana dashboard

Posted by GitBox <gi...@apache.org>.
tysonjh commented on a change in pull request #12499:
URL: https://github.com/apache/beam/pull/12499#discussion_r467182070



##########
File path: .test-infra/jenkins/job_LoadTests_ParDo_Flink_Python.groovy
##########
@@ -161,12 +164,13 @@ def streamingScenarios = { datasetName ->
       test           : 'apache_beam.testing.load_tests.pardo_test',
       runner         : CommonTestProperties.Runner.PORTABLE,
       pipelineOptions: [
-        job_name             : 'load-tests-python-flink-streaming-pardo-5-' + now,
+        job_name             : 'load-tests-python-flink-streaming-pardo-1-' + now,
         project              : 'apache-beam-testing',
         publish_to_big_query : true,
         metrics_dataset      : datasetName,
+        // Keep the old name to not break the legacy dashboard

Review comment:
       For my own understanding, could you elaborate on this?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] mxm commented on a change in pull request #12499: [BEAM-10602] Fix load test metrics in Grafana dashboard

Posted by GitBox <gi...@apache.org>.
mxm commented on a change in pull request #12499:
URL: https://github.com/apache/beam/pull/12499#discussion_r469213290



##########
File path: .test-infra/jenkins/job_LoadTests_ParDo_Flink_Python.groovy
##########
@@ -161,12 +164,13 @@ def streamingScenarios = { datasetName ->
       test           : 'apache_beam.testing.load_tests.pardo_test',
       runner         : CommonTestProperties.Runner.PORTABLE,
       pipelineOptions: [
-        job_name             : 'load-tests-python-flink-streaming-pardo-5-' + now,
+        job_name             : 'load-tests-python-flink-streaming-pardo-1-' + now,

Review comment:
       As for the latency / checkpoint duration. I think they are good panels to have which are applicable to many runners. I'd like to keep them where they are so we can follow the performance regression guidelines in the release guide.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] mxm commented on pull request #12499: [BEAM-10602] Fix load test metrics in Grafana dashboard

Posted by GitBox <gi...@apache.org>.
mxm commented on pull request #12499:
URL: https://github.com/apache/beam/pull/12499#issuecomment-682596444


   Thanks @kamilwu. Here is the fix for the two issues you mentioned: https://github.com/apache/beam/pull/12717


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] tysonjh commented on pull request #12499: [BEAM-10602] Fix load test metrics in Grafana dashboard

Posted by GitBox <gi...@apache.org>.
tysonjh commented on pull request #12499:
URL: https://github.com/apache/beam/pull/12499#issuecomment-672113428


   > @tysonjh You should be able to run this locally with the backup data which is automatically retrieved from the GCS bucket when you run `docker-compose up`. Basically, the changes here will restore the old behavior + add a latency/checkpoint panel + combine the Flink ParDo results with the ones from Dataflow/Spark in one panel.
   
   Got it, thanks. I had to add some extra steps to the wiki for setting up the InfluxDB Data Source and now have graphs showing up. Please let me know when the comments are resolved so I can take another look.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] mxm commented on a change in pull request #12499: [BEAM-10602] Fix load test metrics in Grafana dashboard

Posted by GitBox <gi...@apache.org>.
mxm commented on a change in pull request #12499:
URL: https://github.com/apache/beam/pull/12499#discussion_r467944914



##########
File path: .test-infra/jenkins/job_LoadTests_ParDo_Flink_Python.groovy
##########
@@ -161,12 +164,13 @@ def streamingScenarios = { datasetName ->
       test           : 'apache_beam.testing.load_tests.pardo_test',
       runner         : CommonTestProperties.Runner.PORTABLE,
       pipelineOptions: [
-        job_name             : 'load-tests-python-flink-streaming-pardo-5-' + now,
+        job_name             : 'load-tests-python-flink-streaming-pardo-1-' + now,
         project              : 'apache-beam-testing',
         publish_to_big_query : true,
         metrics_dataset      : datasetName,
+        // Keep the old name to not break the legacy dashboard

Review comment:
       This is such that the old dashboard continues to work: https://apache-beam-testing.appspot.com/explore?dashboard=5751884853805056
   
   It's been a bit of a frustrating experience trying to migrate what we have there..




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] kamilwu commented on a change in pull request #12499: [BEAM-10602] Fix load test metrics in Grafana dashboard

Posted by GitBox <gi...@apache.org>.
kamilwu commented on a change in pull request #12499:
URL: https://github.com/apache/beam/pull/12499#discussion_r467970537



##########
File path: .test-infra/jenkins/job_LoadTests_ParDo_Flink_Python.groovy
##########
@@ -161,12 +164,13 @@ def streamingScenarios = { datasetName ->
       test           : 'apache_beam.testing.load_tests.pardo_test',
       runner         : CommonTestProperties.Runner.PORTABLE,
       pipelineOptions: [
-        job_name             : 'load-tests-python-flink-streaming-pardo-5-' + now,
+        job_name             : 'load-tests-python-flink-streaming-pardo-1-' + now,

Review comment:
       > there are no other streaming load tests at the moment.
   
   Not quite. At the moment, we have streaming load tests for Java (Dataflow only). Apart from that, I'm investigating running other Python load tests (ParDo 1, 2, 3 and 4) in streaming mode too.  




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] kamilwu commented on a change in pull request #12499: [BEAM-10602] Fix load test metrics in Grafana dashboard

Posted by GitBox <gi...@apache.org>.
kamilwu commented on a change in pull request #12499:
URL: https://github.com/apache/beam/pull/12499#discussion_r467981012



##########
File path: .test-infra/jenkins/job_LoadTests_ParDo_Flink_Python.groovy
##########
@@ -161,12 +164,13 @@ def streamingScenarios = { datasetName ->
       test           : 'apache_beam.testing.load_tests.pardo_test',
       runner         : CommonTestProperties.Runner.PORTABLE,
       pipelineOptions: [
-        job_name             : 'load-tests-python-flink-streaming-pardo-5-' + now,
+        job_name             : 'load-tests-python-flink-streaming-pardo-1-' + now,

Review comment:
       > If the iterations don't match, we can adjust that. The input is already the same.
   
   @mxm Do you think it is possible to adjust those parameters so that `pardo-5` can become `pardo-1` and `pardo-6` can become `pardo-2`, `pardo-3` or `pardo-4`? The main advantage of this solution is that we wouldn't have to modify dashboards at all. The old version would just work.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] mxm commented on pull request #12499: [BEAM-10602] Fix load test metrics in Grafana dashboard

Posted by GitBox <gi...@apache.org>.
mxm commented on pull request #12499:
URL: https://github.com/apache/beam/pull/12499#issuecomment-672837418


   @tysonjh or @kamilwu Please have another look and let me know. You can do this online now once the changes have been deployed. Note, that for the first ParDo panel, the results for Flink will still have to be populated over the next days.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] mxm commented on a change in pull request #12499: [BEAM-10602] Fix load test metrics in Grafana dashboard

Posted by GitBox <gi...@apache.org>.
mxm commented on a change in pull request #12499:
URL: https://github.com/apache/beam/pull/12499#discussion_r467943622



##########
File path: .test-infra/jenkins/job_LoadTests_ParDo_Flink_Python.groovy
##########
@@ -161,12 +164,13 @@ def streamingScenarios = { datasetName ->
       test           : 'apache_beam.testing.load_tests.pardo_test',
       runner         : CommonTestProperties.Runner.PORTABLE,
       pipelineOptions: [
-        job_name             : 'load-tests-python-flink-streaming-pardo-5-' + now,
+        job_name             : 'load-tests-python-flink-streaming-pardo-1-' + now,

Review comment:
       Note, this is just the job name. More important is the table we are writing to further down. Unfortunately, the Grafana setup forces me to do that. I would rather not change this but the Grafana setup is very inflexible and in this regard a regression from the old framework we used: https://apache-beam-testing.appspot.com/explore?dashboard=5751884853805056
   
   >Your streaming tests are a bit problematic, because they are not being run on Dataflow and batch.
   
   I don't fully understand your point to be honest, in order for the dropdown menus to work properly, i.e. choosing `SDK` and the `mode` (batch/streaming), this change is required because the table name is composed of `$sdk_$mode_`. The test parameters looked identical to me for Dataflow/Flink. If the iterations don't match, we can adjust that. The input is already the same.
   
   Adding more charts would be another option. We have to remove the streaming dropdown and just add one chart per streaming and batch run. I think that is the best option. It gives us a bit more flexibility.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] kamilwu commented on a change in pull request #12499: [BEAM-10602] Fix load test metrics in Grafana dashboard

Posted by GitBox <gi...@apache.org>.
kamilwu commented on a change in pull request #12499:
URL: https://github.com/apache/beam/pull/12499#discussion_r467975137



##########
File path: .test-infra/jenkins/job_LoadTests_ParDo_Flink_Python.groovy
##########
@@ -161,12 +164,13 @@ def streamingScenarios = { datasetName ->
       test           : 'apache_beam.testing.load_tests.pardo_test',
       runner         : CommonTestProperties.Runner.PORTABLE,
       pipelineOptions: [
-        job_name             : 'load-tests-python-flink-streaming-pardo-5-' + now,
+        job_name             : 'load-tests-python-flink-streaming-pardo-1-' + now,

Review comment:
       @kkucharc is actually working on streaming load tests for Python on Dataflow and she's already prepared a PR: https://github.com/apache/beam/pull/12435. We would like to show metrics from these new tests too. 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] mxm commented on pull request #12499: [BEAM-10602] Fix load test metrics in Grafana dashboard

Posted by GitBox <gi...@apache.org>.
mxm commented on pull request #12499:
URL: https://github.com/apache/beam/pull/12499#issuecomment-670609618


   R: @tysonjh 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] kamilwu commented on a change in pull request #12499: [BEAM-10602] Fix load test metrics in Grafana dashboard

Posted by GitBox <gi...@apache.org>.
kamilwu commented on a change in pull request #12499:
URL: https://github.com/apache/beam/pull/12499#discussion_r468011092



##########
File path: .test-infra/jenkins/job_LoadTests_ParDo_Flink_Python.groovy
##########
@@ -161,12 +164,13 @@ def streamingScenarios = { datasetName ->
       test           : 'apache_beam.testing.load_tests.pardo_test',
       runner         : CommonTestProperties.Runner.PORTABLE,
       pipelineOptions: [
-        job_name             : 'load-tests-python-flink-streaming-pardo-5-' + now,
+        job_name             : 'load-tests-python-flink-streaming-pardo-1-' + now,

Review comment:
       > As for pardo_6, that's not possible because it measures the checkpoint duration and should be a separate panel.
   
   I see. Then, let's do the opposite way (adding new charts and removing the selector). Thank you.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] mxm commented on a change in pull request #12499: [BEAM-10602] Fix load test metrics in Grafana dashboard

Posted by GitBox <gi...@apache.org>.
mxm commented on a change in pull request #12499:
URL: https://github.com/apache/beam/pull/12499#discussion_r467943622



##########
File path: .test-infra/jenkins/job_LoadTests_ParDo_Flink_Python.groovy
##########
@@ -161,12 +164,13 @@ def streamingScenarios = { datasetName ->
       test           : 'apache_beam.testing.load_tests.pardo_test',
       runner         : CommonTestProperties.Runner.PORTABLE,
       pipelineOptions: [
-        job_name             : 'load-tests-python-flink-streaming-pardo-5-' + now,
+        job_name             : 'load-tests-python-flink-streaming-pardo-1-' + now,

Review comment:
       Note, this is just the job name. More important is the table we are writing to further down. Unfortunately, the Grafana setup forces me to do that. I would rather not change this but the Grafana setup is very inflexible and in this regard a regression from the old framework we used: https://apache-beam-testing.appspot.com/explore?dashboard=5751884853805056
   
   >Your streaming tests are a bit problematic, because they are not being run on Dataflow and batch.
   
   I don't fully understand your point to be honest, in order for the dropdown menus to work properly, i.e. choosing `SDK` and the `mode` (batch/streaming), this change is required because the table name is composed of `$sdk_$mode_`. The test parameters looked identical to me for Dataflow/Flink. If the iterations don't match, we can adjust that. The input is already the same.
   
   Adding more charts would be another option, though we completely lose the idea of being able to compare Runner performance. We have to remove the streaming dropdown and just add one chart per streaming and batch run. I think that is the best option. It gives us a bit more flexibility.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] mxm edited a comment on pull request #12499: [BEAM-10602] Fix load test metrics in Grafana dashboard

Posted by GitBox <gi...@apache.org>.
mxm edited a comment on pull request #12499:
URL: https://github.com/apache/beam/pull/12499#issuecomment-672836242


   I'm going to merge this to bring back the metrics to the dashboard again. For now, I think the solution is the best. We can think about moving latency/checkpoint duration to a separate dashboard but I think it is best to have all the metrics for the release performance regression check in one place.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] mxm commented on pull request #12499: [BEAM-10602] Fix load test metrics in Grafana dashboard

Posted by GitBox <gi...@apache.org>.
mxm commented on pull request #12499:
URL: https://github.com/apache/beam/pull/12499#issuecomment-671392502


   @tysonjh You should be able to run this locally with the backup data which is automatically retrieved from the GCS bucket when you run `docker-compose up`. Basically, the changes here will restore the old behavior + add a latency/checkpoint panel + combine the Flink ParDo results with the ones from Dataflow/Spark in one panel.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] amaliujia commented on a change in pull request #12499: [BEAM-10602] Fix load test metrics in Grafana dashboard

Posted by GitBox <gi...@apache.org>.
amaliujia commented on a change in pull request #12499:
URL: https://github.com/apache/beam/pull/12499#discussion_r467174717



##########
File path: .test-infra/metrics/grafana/dashboards/perftests_metrics/ParDo_Load_Tests.json
##########
@@ -619,5 +855,5 @@
   "variables": {
     "list": []
   },
-  "version": 2
+  "version": 8
 }

Review comment:
       nit: add a newline?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] mxm commented on a change in pull request #12499: [BEAM-10602] Fix load test metrics in Grafana dashboard

Posted by GitBox <gi...@apache.org>.
mxm commented on a change in pull request #12499:
URL: https://github.com/apache/beam/pull/12499#discussion_r467184772



##########
File path: .test-infra/metrics/grafana/dashboards/perftests_metrics/ParDo_Load_Tests.json
##########
@@ -619,5 +855,5 @@
   "variables": {
     "list": []
   },
-  "version": 2
+  "version": 8
 }

Review comment:
       This file is auto-generated but I could add a newline :)




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] mxm commented on a change in pull request #12499: [BEAM-10602] Fix load test metrics in Grafana dashboard

Posted by GitBox <gi...@apache.org>.
mxm commented on a change in pull request #12499:
URL: https://github.com/apache/beam/pull/12499#discussion_r467948430



##########
File path: .test-infra/jenkins/job_LoadTests_ParDo_Flink_Python.groovy
##########
@@ -161,12 +164,13 @@ def streamingScenarios = { datasetName ->
       test           : 'apache_beam.testing.load_tests.pardo_test',
       runner         : CommonTestProperties.Runner.PORTABLE,
       pipelineOptions: [
-        job_name             : 'load-tests-python-flink-streaming-pardo-5-' + now,
+        job_name             : 'load-tests-python-flink-streaming-pardo-1-' + now,

Review comment:
       @kamilwu If you agree, I'd remove the streaming/batch dropdown and just add a new chart for the streaming mode. I suppose that is a better migration path because there are no other streaming load tests at the moment.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] mxm commented on a change in pull request #12499: [BEAM-10602] Fix load test metrics in Grafana dashboard

Posted by GitBox <gi...@apache.org>.
mxm commented on a change in pull request #12499:
URL: https://github.com/apache/beam/pull/12499#discussion_r467997322



##########
File path: .test-infra/jenkins/job_LoadTests_ParDo_Flink_Python.groovy
##########
@@ -161,12 +164,13 @@ def streamingScenarios = { datasetName ->
       test           : 'apache_beam.testing.load_tests.pardo_test',
       runner         : CommonTestProperties.Runner.PORTABLE,
       pipelineOptions: [
-        job_name             : 'load-tests-python-flink-streaming-pardo-5-' + now,
+        job_name             : 'load-tests-python-flink-streaming-pardo-1-' + now,

Review comment:
       > @mxm Do you think it is possible to adjust those parameters so that `pardo-5` can become `pardo-1` and `pardo-6` can become `pardo-2`, `pardo-3` or `pardo-4`? The main advantage of this solution is that we wouldn't have to modify dashboards at all. The old version would just work.
   
   That was the original idea in this PR which you I understood you didn't like. pardo_5 became pardo_1. As for pardo_6, that's not possible because it measures the checkpoint duration and should be a separate panel.
   
   > If not, then I'm fine with adding new charts (I suppose you'd meant "chart", "dashboard" is a different kind of thing) and removing the selector for batch/streaming.
   
   Yes, I meant panel, corrected above.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] kamilwu commented on pull request #12499: [BEAM-10602] Fix load test metrics in Grafana dashboard

Posted by GitBox <gi...@apache.org>.
kamilwu commented on pull request #12499:
URL: https://github.com/apache/beam/pull/12499#issuecomment-672897466


   Thanks @mxm. Just a couple of thoughts after reviewing the panels:
   
   1. there is something wrong with a legend (fifth and sixth panel affected). It says "$tag_metric" for both data series. This can be fixed by leaving the ALIAS BY field empty. Just in case, here's a documentation that explains how aliasing works: https://grafana.com/docs/grafana/latest/features/datasources/influxdb/#alias-patterns
   2. we should fix parameter values in the title of the fifth panel after adjusting those parameters in job definition
   
   +1 for keeping latency/checkpoint duration where there are now


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] mxm commented on a change in pull request #12499: [BEAM-10602] Fix load test metrics in Grafana dashboard

Posted by GitBox <gi...@apache.org>.
mxm commented on a change in pull request #12499:
URL: https://github.com/apache/beam/pull/12499#discussion_r467976680



##########
File path: .test-infra/jenkins/job_LoadTests_ParDo_Flink_Python.groovy
##########
@@ -161,12 +164,13 @@ def streamingScenarios = { datasetName ->
       test           : 'apache_beam.testing.load_tests.pardo_test',
       runner         : CommonTestProperties.Runner.PORTABLE,
       pipelineOptions: [
-        job_name             : 'load-tests-python-flink-streaming-pardo-5-' + now,
+        job_name             : 'load-tests-python-flink-streaming-pardo-1-' + now,

Review comment:
       Good to hear. Still, there is no streaming data in the dashboards yet :)
   
   So do you agree to add new ~dashboards~ panels for the streaming tests and removing the selector for batch/streaming?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] mxm commented on a change in pull request #12499: [BEAM-10602] Fix load test metrics in Grafana dashboard

Posted by GitBox <gi...@apache.org>.
mxm commented on a change in pull request #12499:
URL: https://github.com/apache/beam/pull/12499#discussion_r469212868



##########
File path: .test-infra/jenkins/job_LoadTests_ParDo_Flink_Python.groovy
##########
@@ -161,12 +164,13 @@ def streamingScenarios = { datasetName ->
       test           : 'apache_beam.testing.load_tests.pardo_test',
       runner         : CommonTestProperties.Runner.PORTABLE,
       pipelineOptions: [
-        job_name             : 'load-tests-python-flink-streaming-pardo-5-' + now,
+        job_name             : 'load-tests-python-flink-streaming-pardo-1-' + now,

Review comment:
       I went the other route you suggested and adjusted the parameters for the load tests. Adding more panels seemed like a good idea but it also adds significant noise to the dashboard.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] mxm commented on pull request #12499: [BEAM-10602] Fix load test metrics in Grafana dashboard

Posted by GitBox <gi...@apache.org>.
mxm commented on pull request #12499:
URL: https://github.com/apache/beam/pull/12499#issuecomment-672836242


   I'm going to merge this to bring back the metrics to the dashboard again. For now, I think the solution is the best. We can think about moving latency/checkpoint duration to a separate dashboard but I think it is best to have all the metrics for the release performance regression check in place.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] kamilwu commented on a change in pull request #12499: [BEAM-10602] Fix load test metrics in Grafana dashboard

Posted by GitBox <gi...@apache.org>.
kamilwu commented on a change in pull request #12499:
URL: https://github.com/apache/beam/pull/12499#discussion_r467985468



##########
File path: .test-infra/jenkins/job_LoadTests_ParDo_Flink_Python.groovy
##########
@@ -161,12 +164,13 @@ def streamingScenarios = { datasetName ->
       test           : 'apache_beam.testing.load_tests.pardo_test',
       runner         : CommonTestProperties.Runner.PORTABLE,
       pipelineOptions: [
-        job_name             : 'load-tests-python-flink-streaming-pardo-5-' + now,
+        job_name             : 'load-tests-python-flink-streaming-pardo-1-' + now,

Review comment:
       If not, then I'm fine with adding new charts (I suppose you'd meant "chart", "dashboard" is a different kind of thing) and removing the selector for batch/streaming.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] mxm merged pull request #12499: [BEAM-10602] Fix load test metrics in Grafana dashboard

Posted by GitBox <gi...@apache.org>.
mxm merged pull request #12499:
URL: https://github.com/apache/beam/pull/12499


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] mxm commented on a change in pull request #12499: [BEAM-10602] Fix load test metrics in Grafana dashboard

Posted by GitBox <gi...@apache.org>.
mxm commented on a change in pull request #12499:
URL: https://github.com/apache/beam/pull/12499#discussion_r467976680



##########
File path: .test-infra/jenkins/job_LoadTests_ParDo_Flink_Python.groovy
##########
@@ -161,12 +164,13 @@ def streamingScenarios = { datasetName ->
       test           : 'apache_beam.testing.load_tests.pardo_test',
       runner         : CommonTestProperties.Runner.PORTABLE,
       pipelineOptions: [
-        job_name             : 'load-tests-python-flink-streaming-pardo-5-' + now,
+        job_name             : 'load-tests-python-flink-streaming-pardo-1-' + now,

Review comment:
       Good to hear. Still, there is no streaming data in the dashboards yet :)
   
   So do you agree to add new dashboards for the streaming tests and removing the selector for batch/streaming?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] kamilwu commented on a change in pull request #12499: [BEAM-10602] Fix load test metrics in Grafana dashboard

Posted by GitBox <gi...@apache.org>.
kamilwu commented on a change in pull request #12499:
URL: https://github.com/apache/beam/pull/12499#discussion_r467907301



##########
File path: .test-infra/jenkins/job_LoadTests_ParDo_Flink_Python.groovy
##########
@@ -161,12 +164,13 @@ def streamingScenarios = { datasetName ->
       test           : 'apache_beam.testing.load_tests.pardo_test',
       runner         : CommonTestProperties.Runner.PORTABLE,
       pipelineOptions: [
-        job_name             : 'load-tests-python-flink-streaming-pardo-5-' + now,
+        job_name             : 'load-tests-python-flink-streaming-pardo-1-' + now,

Review comment:
       I have mixed feelings about it. This test is not `load-tests-python-flink-batch-pardo-1` but on streaming. There are more differences between them: batch-pardo-1 uses 10 iterations, this test uses 5 iterations. 0 counters in batch-pardo-1 vs. 3 counters right here. Because of that, I think we should stay with the previous job_name: `load-tests-python-flink-streaming-pardo-5`.
   
   The general idea behind load tests is that we run the same configuration on different runners, in different SDKs and in different mode (batch or streaming). Grafana dashboards for load tests were designed with that convention in mind. If you choose `java` and `streaming` from the list, Grafana will pull data from these measurements: `java_streaming_pardo_1`, `java_streaming_pardo_2` and so. Your streaming tests are a bit problematic, because they are not being run on Dataflow and batch. Also, they have no Java counterpart.
   
   That being said, I think about two solutions:
   1) Add more charts. We would end up with a total of six charts. The fifth and the sixth chart would be empty in most cases (for Java and for batch).
   2) Create a separate, more specific version of dashboard just for these two tests (streaming-pardo-5 and streaming-pardo-6). Leave "ParDo Load Tests" dashboard intact.
   
   @mxm What do you think?
   
   
   




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org