Surya Hebbar has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20355 )

Change subject: IMPALA-12364: Display memory, disk and network metrics in 
webUI's query timeline
......................................................................


Patch Set 3:

(9 comments)

The gridlines and tooltips in all the charts are shown while hovering. Both the 
charts contain a close button. Also, the code's redundancy has been reduced.

http://gerrit.cloudera.org:8080/#/c/20355/1/www/query_timeline.tmpl
File www/query_timeline.tmpl:

http://gerrit.cloudera.org:8080/#/c/20355/1/www/query_timeline.tmpl@966
PS1, Line 966:  if (max_samples_utilization.available <= 2) {
             :       var utilization_samples_message = `Warning: Not enough 
samples for
             :           CPU utilization plot. Please decrease the value of 
starting flag
             :           variable <b><i>periodic_counter_update_period_ms 
</b></i> to
             :           increase the granularity of CPU utilization plot.`;
             :       try {
             :         host_utilization_chart = 
host_utilization_chart.destroy();
             :       } catch (e) {
             :         host_utilization_chart = null;
             :       }
             :       displayWarning(host_utilization_diagram, 
utilization_samples_message);
             :       utilization_metrics_parse_successful = true;
             :       return;
             :     }
             :     cpu
> It will be nice to to separate the profile walk part (extracting the sample
Thanks, I have separated common the walk part.


http://gerrit.cloudera.org:8080/#/c/20355/1/www/query_timeline.tmpl@1001
PS1, Line 1001:   clearTimeseriesValues(acc_usage, max_samples_utilization);
              :     });
              :     read_write_metrics_aggregate.forEach(function (acc_usage) {
              :       clearTimeseriesValues(acc_usage, max_samples_utilization);
              :     });
              :     clearTimesamples(sampled_utilization_timeseries, 
max_samples_utilization);
              :     prev_utilization_num_samples = 
profile.child_profiles[1].summary_stats_counters[0]
              :         .num_of_samples;
              :     utilization_metrics_parse_successful = true;
              :   } catch (e) {
              :     u
> Is this code meant to normalize the data samples?
Yes. It's supposed to normalize the samples.

I have created a function for it now.


http://gerrit.cloudera.org:8080/#/c/20355/2/www/query_timeline.tmpl
File www/query_timeline.tmpl:

http://gerrit.cloudera.org:8080/#/c/20355/2/www/query_timeline.tmpl@65
PS2, Line 65:       <h4 style="display:inline;"> Download : </h4>
> Need a way to hide the fragment chart too. Maybe clocking on the fragment c
Yes. I was planning to do it in the subsequent patch set and have done it.

Now, the fragment chart and utilization chart both contain a close button.


http://gerrit.cloudera.org:8080/#/c/20355/2/www/query_timeline.tmpl@469
PS2, Line 469:           for (var cpn = 1; cpn < fp.child_profiles.length; 
++cpn) {
> Timing height bound should not go below 200:
I have set the minimum height for all the diagrams as 200.


http://gerrit.cloudera.org:8080/#/c/20355/2/www/query_timeline.tmpl@670
PS2, Line 670:   });
> Change to:
Done


http://gerrit.cloudera.org:8080/#/c/20355/2/www/query_timeline.tmpl@674
PS2, Line 674: }
> Math.min(window.innerHeight * 0.3, 200);
Done


http://gerrit.cloudera.org:8080/#/c/20355/2/www/query_timeline.tmpl@695
PS2, Line 695:
> This should not be B units for Memory, not B/s rates.
Done


http://gerrit.cloudera.org:8080/#/c/20355/2/www/query_timeline.tmpl@766
PS2, Line 766:           format : function (x) { return x.toFixed(decimals); }
> These should be mapped by label rather than position.
Done


http://gerrit.cloudera.org:8080/#/c/20355/2/www/query_timeline.tmpl@1151
PS2, Line 1151:
> Zooming out in the browser (WITH ctrl-'-') then back in (ctrl-'+') is not c
After providing the horizontal zoom option, only the height of charts and 
diagrams is being resized according to the zoom.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifd25e6f0bc9fbd664ec98936daff3f27182dfc7f
Gerrit-Change-Number: 20355
Gerrit-PatchSet: 3
Gerrit-Owner: Surya Hebbar <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Kurt Deschler <[email protected]>
Gerrit-Reviewer: Riza Suminto <[email protected]>
Gerrit-Reviewer: Surya Hebbar <[email protected]>
Gerrit-Reviewer: Wenzhe Zhou <[email protected]>
Gerrit-Comment-Date: Tue, 22 Aug 2023 14:24:31 +0000
Gerrit-HasComments: Yes

Reply via email to