You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Surya Hebbar (Code Review)" <ge...@cloudera.org> on 2024/07/18 13:47:18 UTC

[Impala-ASF-CR] IMPALA-12576: Display warning on the query timeline page for the experimental profile

Surya Hebbar has uploaded this change for review. ( http://gerrit.cloudera.org:8080/21596


Change subject: IMPALA-12576: Display warning on the query timeline page for the experimental profile
......................................................................

IMPALA-12576: Display warning on the query timeline page for the experimental profile

On switching from the traditional to the experimental profile
by setting 'gen_experimental_profile' to 'true', the fragment diagram
on the query timeline page breaks.

Currently, the experimental profile does not contain the instance level
event details required for rendering the fragment timing diagram.
These have been deliberately kept out from experimental profile for
particular reasons, which are attached to IMPALA-12576.

A warning message is displayed in the place of fragment timing diagram,
whenever experimental profile is being used.

The utilization chart below it will be displayed as before, without any
changes.

Previously generated experimental profiles that have been imported
to the queries page will also be shown with the same warning.

Change-Id: I6ad62be3572b7a126ce10dc4a11fcc9b8a58013d
---
M www/query_timeline.tmpl
M www/scripts/query_timeline/fragment_diagram.js
M www/scripts/query_timeline/fragment_metrics_diagram.js
M www/scripts/query_timeline/host_utilization_diagram.js
4 files changed, 29 insertions(+), 16 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/96/21596/1
-- 
To view, visit http://gerrit.cloudera.org:8080/21596
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I6ad62be3572b7a126ce10dc4a11fcc9b8a58013d
Gerrit-Change-Number: 21596
Gerrit-PatchSet: 1
Gerrit-Owner: Surya Hebbar <sh...@cloudera.com>

[Impala-ASF-CR] IMPALA-12576: Display warning on the query timeline page for the experimental profile

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

Change subject: IMPALA-12576: Display warning on the query timeline page for the experimental profile
......................................................................


Patch Set 2:

Is it possible to add an automated test? Not having automated tests for WebUI features can cause these tickets to get flagged as missing test coverage.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6ad62be3572b7a126ce10dc4a11fcc9b8a58013d
Gerrit-Change-Number: 21596
Gerrit-PatchSet: 2
Gerrit-Owner: Surya Hebbar <sh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Surya Hebbar <sh...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Thu, 15 Aug 2024 15:25:47 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12576: Display warning on the query timeline page for the experimental profile

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

Change subject: IMPALA-12576: Display warning on the query timeline page for the experimental profile
......................................................................


Patch Set 2:

(1 comment)

Thanks for fixing this!

http://gerrit.cloudera.org:8080/#/c/21596/2/www/scripts/query_timeline/fragment_diagram.js
File www/scripts/query_timeline/fragment_diagram.js:

http://gerrit.cloudera.org:8080/#/c/21596/2/www/scripts/query_timeline/fragment_diagram.js@169
PS2, Line 169: resizeTimingDiagramWarning
I think the height can be adjusted as well. It looks too high right now with a lot of empty space. 50px seems appropriate.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6ad62be3572b7a126ce10dc4a11fcc9b8a58013d
Gerrit-Change-Number: 21596
Gerrit-PatchSet: 2
Gerrit-Owner: Surya Hebbar <sh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Fri, 19 Jul 2024 09:10:15 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12576: Display warning on the query timeline page for the experimental profile

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

Change subject: IMPALA-12576: Display warning on the query timeline page for the experimental profile
......................................................................


Patch Set 2:

The patch to implement aggregate metrics within experimental profile is already on the way.

So, this patch may not be much helpful except for maybe including as a fix in previous versions.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6ad62be3572b7a126ce10dc4a11fcc9b8a58013d
Gerrit-Change-Number: 21596
Gerrit-PatchSet: 2
Gerrit-Owner: Surya Hebbar <sh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Surya Hebbar <sh...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Thu, 22 Aug 2024 10:21:08 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12576: Display warning on the query timeline page for the experimental profile

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

Change subject: IMPALA-12576: Display warning on the query timeline page for the experimental profile
......................................................................


Patch Set 2:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/21596/2/www/scripts/query_timeline/fragment_diagram.js
File www/scripts/query_timeline/fragment_diagram.js:

http://gerrit.cloudera.org:8080/#/c/21596/2/www/scripts/query_timeline/fragment_diagram.js@351
PS2, Line 351: execution_profile.child_profiles[1].profile_name.startsWith("Averaged")
Do we print TRuntimeProfileTree.profile_version in json profile format today?
We probably should start doing that for easy distinction. Feel free to file a follow up JIRA.


http://gerrit.cloudera.org:8080/#/c/21596/2/www/scripts/query_timeline/fragment_diagram.js@352
PS2, Line 352: Warning: Instance level details are
             :       unavailable, when <b><i>gen_experimental_profile</i></b> is set to
             :       <b><i>true</i></b>.
I'd like to suggest rewording this to:

`Warning: Instance level details are unavailable. Query profile might be in Version 2 format (<b><i>gen_experimental_profile</i></b> flag is set to <b><i>true</i></b>).`


http://gerrit.cloudera.org:8080/#/c/21596/2/www/scripts/query_timeline/fragment_metrics_diagram.js
File www/scripts/query_timeline/fragment_metrics_diagram.js:

http://gerrit.cloudera.org:8080/#/c/21596/2/www/scripts/query_timeline/fragment_metrics_diagram.js@202
PS2, Line 202: "Warning: Not enough samples for fragment metrics plot."
Unnecessary change?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6ad62be3572b7a126ce10dc4a11fcc9b8a58013d
Gerrit-Change-Number: 21596
Gerrit-PatchSet: 2
Gerrit-Owner: Surya Hebbar <sh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Surya Hebbar <sh...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Thu, 22 Aug 2024 16:05:19 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12576: Display warning on the query timeline page for the experimental profile

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

Change subject: IMPALA-12576: Display warning on the query timeline page for the experimental profile
......................................................................


Patch Set 2:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/16538/ : 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/21596
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6ad62be3572b7a126ce10dc4a11fcc9b8a58013d
Gerrit-Change-Number: 21596
Gerrit-PatchSet: 2
Gerrit-Owner: Surya Hebbar <sh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Thu, 18 Jul 2024 14:45:07 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12576: Display warning on the query timeline page for the experimental profile

Posted by "Surya Hebbar (Code Review)" <ge...@cloudera.org>.
Surya Hebbar has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/21596 )

Change subject: IMPALA-12576: Display warning on the query timeline page for the experimental profile
......................................................................

IMPALA-12576: Display warning on the query timeline page for the experimental profile

On switching from the traditional to the experimental profile
by setting 'gen_experimental_profile' to 'true', the fragment diagram
on the query timeline page breaks.

Currently, the experimental profile does not contain the instance level
event details required for rendering the fragment timing diagram.
These have been deliberately kept out from experimental profile for
particular reasons, which are attached to IMPALA-12576.

A warning message is displayed in the place of fragment timing diagram,
whenever experimental profile is being used.

The utilization chart below it will be displayed as before, without any
changes.

Previously generated experimental profiles that have been imported
to the queries page will also be shown with the same warning.

Change-Id: I6ad62be3572b7a126ce10dc4a11fcc9b8a58013d
---
M www/query_timeline.tmpl
M www/scripts/query_timeline/fragment_diagram.js
M www/scripts/query_timeline/fragment_metrics_diagram.js
M www/scripts/query_timeline/host_utilization_diagram.js
4 files changed, 29 insertions(+), 16 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6ad62be3572b7a126ce10dc4a11fcc9b8a58013d
Gerrit-Change-Number: 21596
Gerrit-PatchSet: 2
Gerrit-Owner: Surya Hebbar <sh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>

[Impala-ASF-CR] IMPALA-12576: Display warning on the query timeline page for the experimental profile

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

Change subject: IMPALA-12576: Display warning on the query timeline page for the experimental profile
......................................................................


Patch Set 2:

(1 comment)

Thank you for your comments. Please let me know if any other changes are required

http://gerrit.cloudera.org:8080/#/c/21596/2/www/scripts/query_timeline/fragment_diagram.js
File www/scripts/query_timeline/fragment_diagram.js:

http://gerrit.cloudera.org:8080/#/c/21596/2/www/scripts/query_timeline/fragment_diagram.js@169
PS2, Line 169: resizeTimingDiagramWarning
> I think the height can be adjusted as well. It looks too high right now wit
The height is not being controlled here. It is actually part of the 'displayWarning' function.

It was kept as '200px' as it was the minimum height for all charts on the page, also for other reasons related to resizing.

Please let me know if it has other implications.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6ad62be3572b7a126ce10dc4a11fcc9b8a58013d
Gerrit-Change-Number: 21596
Gerrit-PatchSet: 2
Gerrit-Owner: Surya Hebbar <sh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Surya Hebbar <sh...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Thu, 01 Aug 2024 06:53:38 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12576: Display warning on the query timeline page for the experimental profile

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

Change subject: IMPALA-12576: Display warning on the query timeline page for the experimental profile
......................................................................


Patch Set 1:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/16537/ : 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/21596
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6ad62be3572b7a126ce10dc4a11fcc9b8a58013d
Gerrit-Change-Number: 21596
Gerrit-PatchSet: 1
Gerrit-Owner: Surya Hebbar <sh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Thu, 18 Jul 2024 14:12:21 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12576: Display warning on the query timeline page for the experimental profile

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

Change subject: IMPALA-12576: Display warning on the query timeline page for the experimental profile
......................................................................


Patch Set 2:

Currently, the "collectFragmentEventsFromProfile" method is too large for implementing a test, while mocking the entire surrounding variables and environment.

But, there are already tests written for functions parsing the profile and collecting metrics here.
https://gerrit.cloudera.org/c/20538/

Using JEST testing framework to test this would be difficult as I mentioned. It might be possible to add python webUI tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6ad62be3572b7a126ce10dc4a11fcc9b8a58013d
Gerrit-Change-Number: 21596
Gerrit-PatchSet: 2
Gerrit-Owner: Surya Hebbar <sh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Surya Hebbar <sh...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Thu, 22 Aug 2024 10:31:11 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12576: Display warning on the query timeline page for the experimental profile

Posted by "Surya Hebbar (Code Review)" <ge...@cloudera.org>.
Surya Hebbar has abandoned this change. ( http://gerrit.cloudera.org:8080/21596 )

Change subject: IMPALA-12576: Display warning on the query timeline page for the experimental profile
......................................................................


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: abandon
Gerrit-Change-Id: I6ad62be3572b7a126ce10dc4a11fcc9b8a58013d
Gerrit-Change-Number: 21596
Gerrit-PatchSet: 2
Gerrit-Owner: Surya Hebbar <sh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Surya Hebbar <sh...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>