Riza Suminto 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 3: (3 comments) http://gerrit.cloudera.org:8080/#/c/23068/3/www/scripts/query_timeline/fragment_diagram.js File www/scripts/query_timeline/fragment_diagram.js: http://gerrit.cloudera.org:8080/#/c/23068/3/www/scripts/query_timeline/fragment_diagram.js@34 PS3, Line 34: export let legacy_profile = false; Use profile_version from TRuntimeProfileTree instead, like export let profile_version = 2; FYI, struct TRuntimeProfileTree { 1: required list<TRuntimeProfileNode> nodes 2: optional ExecStats.TExecSummary exec_summary // The version of the runtime profile representation. Different versions may have // different invariants or information. Thrift structures for new versions need to // remain readable by readers with old versions of the thrift file, but may remove // information. // Version 1: this field is unset // Version 2: this field is set to 2 // TODO: IMPALA-9846: document which versions of Impala generate which version. 3: optional i32 profile_version } http://gerrit.cloudera.org:8080/#/c/23068/3/www/scripts/query_timeline/fragment_diagram.js@595 PS3, Line 595: export function setTimingDiagramDimensions(ignored_arg) { : // phashes header and timeticks footer with margin and border by considering the offset : page_additional_height = timing_diagram.offsetTop + (ROW_HEIGHT + MARGIN_HEADER_FOOTER : + BORDER_STROKE_WIDTH * 4) * 2; : const FRAGMENT_DIAGRAM_INITIAL_HEIGHT = window.innerHeight - page_additional_height; : const REMAINING_HEIGHT = FRAGMENT_DIAGRAM_INITIAL_HEIGHT - getUtilizationWrapperHeight() : - getFragmentMetricsWrapperHeight(); : const DISPLAY_HEIGHT = Math.max(DIAGRAM_MIN_HEIGHT, Math.min(REMAINING_HEIGHT, : rownum * ROW_HEIGHT)); : chart_width = diagram_width - name_width - MARGIN_CHART_END - BORDER_STROKE_WIDTH; : : phases_header.style.height = `${ROW_HEIGHT}px`; : fragment_diagram.parentElement.style.height = `${DISPLAY_HEIGHT}px`; : fragment_diagram.style.height = `${Math.max(DIAGRAM_MIN_HEIGHT, : rownum * ROW_HEIGHT)}px`; : timeticks_footer.style.height = `${ROW_HEIGHT}px`; : : fragment_diagram.parentElement.style.width = `${diagram_width}px`; : phases_header.parentElement.style.width = `${diagram_width}px`; : timeticks_footer.parentElement.style.width = `${diagram_width}px`; : timing_diagram.style.width = `${diagram_width}px`; : : fragment_diagram.style.width = `${diagram_width}px`; : phases_header.style.width = `${diagram_width}px`; : timeticks_footer.style.width = `${diagram_width}px`; : } : : export function renderTimingDiagram() { : if (fragment_events_parse_successful) { : setTimingDiagramDimensions(); : renderPhases(); : if (legacy_profile) { : renderFragmentDiagram_legacy(); : } else { : renderFragmentDiagram(); : } : renderTimeticks(); : } : } nit: is it necessary to move around functions from its previous locations? It makes code review difficult. http://gerrit.cloudera.org:8080/#/c/23068/3/www/scripts/query_timeline/fragment_diagram.js@720 PS3, Line 720: _legacy Avoid naming with "legacy" suffix or prefix. Instead, use _v1 or _v2. -- 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: 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-Comment-Date: Tue, 08 Jul 2025 18:07:50 +0000 Gerrit-HasComments: Yes
