You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Laszlo Gaal (Code Review)" <ge...@cloudera.org> on 2018/10/04 15:40:02 UTC

[Impala-ASF-CR] Prettify the timeline produced by test-with-docker.py

Laszlo Gaal has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11578


Change subject: Prettify the timeline produced by test-with-docker.py
......................................................................

Prettify the timeline produced by test-with-docker.py

The change tweaks the HTML template for the timeline summary
to make it slightly more readable:
- Adds legend strings to the CPU graphs
- Inserts the test run name into the CPU chart title to clarify
  which chart show which build/test phase
- Stretches the CPU charts a bit wider
- Identifes the common prefix of the phase/container names (the build
  name) and delete it from the chart labels. This increases legibility
  by cutting down on noise and growing the chart real estate.

  To support this change the Python drivers are also changed:
  the build name parameter, which is the common prefix, is passed
  to monitor.py and written to the JSON output

- The name of the build and data load phase container is suffixed with
  "-build" so that it shares the naming convention for the other
  containers.
- The timeline graph section is sized explicitly byt computing the height
  from the number of distinct tasks. This avoids having a second scrollbar
  for the timeline, which is annoying.
  The formula is pretty crude: it uses empirical constants, but produces
  an OK layout for the default font sizes in Chrome (both on Linux
  and the Mac).

Tested so far by tweaking the HTML template and an HTML result file
from an earlier build.

Change-Id: I7a41bea762b0e33f3d71b0be57eedbacb19c680c
---
M docker/monitor.py
M docker/test-with-docker.py
M docker/timeline.html.template
3 files changed, 46 insertions(+), 13 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/78/11578/2
-- 
To view, visit http://gerrit.cloudera.org:8080/11578
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I7a41bea762b0e33f3d71b0be57eedbacb19c680c
Gerrit-Change-Number: 11578
Gerrit-PatchSet: 2
Gerrit-Owner: Laszlo Gaal <la...@cloudera.com>

[Impala-ASF-CR] Prettify the timeline produced by test-with-docker.py

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11578 )

Change subject: Prettify the timeline produced by test-with-docker.py
......................................................................


Patch Set 3:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3295/ DRY_RUN=false


-- 
To view, visit http://gerrit.cloudera.org:8080/11578
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7a41bea762b0e33f3d71b0be57eedbacb19c680c
Gerrit-Change-Number: 11578
Gerrit-PatchSet: 3
Gerrit-Owner: Laszlo Gaal <la...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Laszlo Gaal <la...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Comment-Date: Tue, 09 Oct 2018 15:22:08 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] Prettify the timeline produced by test-with-docker.py

Posted by "Philip Zeyliger (Code Review)" <ge...@cloudera.org>.
Philip Zeyliger has posted comments on this change. ( http://gerrit.cloudera.org:8080/11578 )

Change subject: Prettify the timeline produced by test-with-docker.py
......................................................................


Patch Set 2: Code-Review+2

I think you could move removing a common prefix from the names into Python by adding "common_prefix=..." to Timeline.__init__() or even stripping it when we do the monitoring. That said, it's not a big deal, and the layers are pretty tightly coupled, so I'm fine with the way you did it too.

Thanks for the improvement!


-- 
To view, visit http://gerrit.cloudera.org:8080/11578
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7a41bea762b0e33f3d71b0be57eedbacb19c680c
Gerrit-Change-Number: 11578
Gerrit-PatchSet: 2
Gerrit-Owner: Laszlo Gaal <la...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Comment-Date: Thu, 04 Oct 2018 19:03:25 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] Prettify the timeline produced by test-with-docker.py

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11578 )

Change subject: Prettify the timeline produced by test-with-docker.py
......................................................................


Patch Set 3: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/11578
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7a41bea762b0e33f3d71b0be57eedbacb19c680c
Gerrit-Change-Number: 11578
Gerrit-PatchSet: 3
Gerrit-Owner: Laszlo Gaal <la...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Laszlo Gaal <la...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Comment-Date: Tue, 09 Oct 2018 15:22:07 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] Prettify the timeline produced by test-with-docker.py

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11578 )

Change subject: Prettify the timeline produced by test-with-docker.py
......................................................................

Prettify the timeline produced by test-with-docker.py

The change tweaks the HTML template for the timeline summary
to make it slightly more readable:
- Adds legend strings to the CPU graphs
- Inserts the test run name into the CPU chart title to clarify
  which chart show which build/test phase
- Stretches the CPU charts a bit wider
- Identifes the common prefix of the phase/container names (the build
  name) and delete it from the chart labels. This increases legibility
  by cutting down on noise and growing the chart real estate.

  To support this change the Python drivers are also changed:
  the build name parameter, which is the common prefix, is passed
  to monitor.py and written to the JSON output

- The name of the build and data load phase container is suffixed with
  "-build" so that it shares the naming convention for the other
  containers.
- The timeline graph section is sized explicitly byt computing the height
  from the number of distinct tasks. This avoids having a second scrollbar
  for the timeline, which is annoying.
  The formula is pretty crude: it uses empirical constants, but produces
  an OK layout for the default font sizes in Chrome (both on Linux
  and the Mac).

Tested so far by tweaking the HTML template and an HTML result file
from an earlier build.

Change-Id: I7a41bea762b0e33f3d71b0be57eedbacb19c680c
Reviewed-on: http://gerrit.cloudera.org:8080/11578
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M docker/monitor.py
M docker/test-with-docker.py
M docker/timeline.html.template
3 files changed, 46 insertions(+), 13 deletions(-)

Approvals:
  Impala Public Jenkins: Looks good to me, approved; Verified

-- 
To view, visit http://gerrit.cloudera.org:8080/11578
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I7a41bea762b0e33f3d71b0be57eedbacb19c680c
Gerrit-Change-Number: 11578
Gerrit-PatchSet: 4
Gerrit-Owner: Laszlo Gaal <la...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Laszlo Gaal <la...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>

[Impala-ASF-CR] Prettify the timeline produced by test-with-docker.py

Posted by "Laszlo Gaal (Code Review)" <ge...@cloudera.org>.
Laszlo Gaal has posted comments on this change. ( http://gerrit.cloudera.org:8080/11578 )

Change subject: Prettify the timeline produced by test-with-docker.py
......................................................................


Patch Set 2:

I have considered removing the common prefix, but decided against it. My main reason was that it can still serve as a unique ID for the run. If we ever want to analyze these metrics in a wider context (e.g. comparing runs, looking at trends) the unique ID would still be there. The fact that it is timestamp-based could also have advantages -- and it doesn't take up a lot of space either.


-- 
To view, visit http://gerrit.cloudera.org:8080/11578
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7a41bea762b0e33f3d71b0be57eedbacb19c680c
Gerrit-Change-Number: 11578
Gerrit-PatchSet: 2
Gerrit-Owner: Laszlo Gaal <la...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Laszlo Gaal <la...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Comment-Date: Thu, 04 Oct 2018 20:31:31 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] Prettify the timeline produced by test-with-docker.py

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11578 )

Change subject: Prettify the timeline produced by test-with-docker.py
......................................................................


Patch Set 3: Verified+1


-- 
To view, visit http://gerrit.cloudera.org:8080/11578
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7a41bea762b0e33f3d71b0be57eedbacb19c680c
Gerrit-Change-Number: 11578
Gerrit-PatchSet: 3
Gerrit-Owner: Laszlo Gaal <la...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Laszlo Gaal <la...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Comment-Date: Tue, 09 Oct 2018 19:12:48 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] Prettify the timeline produced by test-with-docker.py

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11578 )

Change subject: Prettify the timeline produced by test-with-docker.py
......................................................................


Patch Set 2:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/937/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/11578
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7a41bea762b0e33f3d71b0be57eedbacb19c680c
Gerrit-Change-Number: 11578
Gerrit-PatchSet: 2
Gerrit-Owner: Laszlo Gaal <la...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Comment-Date: Thu, 04 Oct 2018 16:11:26 +0000
Gerrit-HasComments: No