Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/23068 )
Change subject: IMPALA-13474: Support aggregated profile for webUI's query timeline ...................................................................... Patch Set 7: (5 comments) http://gerrit.cloudera.org:8080/#/c/23068/7//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/23068/7//COMMIT_MSG@24 PS7, Line 24: The fragment's time series counters are currently excluded from the : JSON profile as they are too large.(i.e. "MemoryUsage", ThreadUsage"). : Hence, per fragment usage metrics are currently not supported. Is this still true/are there plans to add this info? http://gerrit.cloudera.org:8080/#/c/23068/7/www/query_profile.tmpl File www/query_profile.tmpl: http://gerrit.cloudera.org:8080/#/c/23068/7/www/query_profile.tmpl@68 PS7, Line 68: if (arr[i] >= 0 && arr[i] <= min_val) This looks at non-negative results, so minNonZero seems misleading http://gerrit.cloudera.org:8080/#/c/23068/7/www/query_profile.tmpl@137 PS7, Line 137: if (event.timestamp) { Can you add some comment about when we'll have timestamp / ts_list / ts_stat in the event? http://gerrit.cloudera.org:8080/#/c/23068/7/www/query_profile.tmpl@141 PS7, Line 141: `${event_sequences}${formatNanoseconds(event.ts_list[ts_i])}, `; Is this an efficient way to concatenate strings in JS? maybe += or joining the array would be better? http://gerrit.cloudera.org:8080/#/c/23068/7/www/scripts/query_timeline/fragment_diagram.js File www/scripts/query_timeline/fragment_diagram.js: http://gerrit.cloudera.org:8080/#/c/23068/7/www/scripts/query_timeline/fragment_diagram.js@641 PS7, Line 641: }); Is there a reason for moving the event listeners here? It would be a bit easier to process the review if they were kept at the end. -- To view, visit http://gerrit.cloudera.org:8080/23068 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If1f59bc8ab6bee91bd5fa446a207891453600b3e Gerrit-Change-Number: 23068 Gerrit-PatchSet: 7 Gerrit-Owner: Surya Hebbar <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Kurt Deschler <[email protected]> Gerrit-Reviewer: Pranav Lodha <[email protected]> Gerrit-Reviewer: Riza Suminto <[email protected]> Gerrit-Reviewer: Surya Hebbar <[email protected]> Gerrit-Comment-Date: Thu, 13 Nov 2025 16:30:20 +0000 Gerrit-HasComments: Yes
