Aleksandr Efimov has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/24353 )

Change subject: IMPALA-12956: Add profile summary output
......................................................................


Patch Set 7:

(7 comments)

PS7 addresses the latest review comments.

Validation:
- git diff --check
- ninja impala-profile-tool unifiedbetests
- bin/run-jvm-binary.sh be/build/latest/service/unifiedbetests 
--gtest_filter=PrintTableTest.*:ProgressToStringTest.*
- bin/impala-py.test -q tests/observability/test_profile_tool.py

http://gerrit.cloudera.org:8080/#/c/24353/7/be/src/util/impala-profile-tool.cc
File be/src/util/impala-profile-tool.cc:

http://gerrit.cloudera.org:8080/#/c/24353/7/be/src/util/impala-profile-tool.cc@84
PS7, Line 84: DEFINE_int32(summary_text_length, 250,
Done in PS7. Renamed the flag and help text to --summary_text_length and 
updated the summary tests accordingly.


http://gerrit.cloudera.org:8080/#/c/24353/7/be/src/util/impala-profile-tool.cc@251
PS7, Line 251:   while (node_idx < tree.nodes.size()) {
Done in PS7.


http://gerrit.cloudera.org:8080/#/c/24353/7/be/src/util/impala-profile-tool.cc@280
PS7, Line 280:       return true;
Done in PS7. ParseTimestamp() now truncates fractional seconds to microseconds 
and uses boost::posix_time::time_from_string() / ptime.


http://gerrit.cloudera.org:8080/#/c/24353/7/be/src/util/impala-profile-tool.cc@484
PS7, Line 484:     return ProgressToString(completed_scan_ranges, 
completed_scan_ranges);
Done in PS7. Removed the Estimated Per-Host Mem fallback; summary output now 
reports Cluster Memory Admitted or N/A.


http://gerrit.cloudera.org:8080/#/c/24353/7/be/src/util/impala-profile-tool.cc@495
PS7, Line 495:       && num_fragment_instances > 0) {
Done in PS7. Removed the live TExecSummary.progress fallback; archived 
failed/cancelled fixture tests now expect N/A when the archived counters are 
unavailable.


http://gerrit.cloudera.org:8080/#/c/24353/7/be/src/util/impala-profile-tool.cc@593
PS7, Line 593:       && profile_format != "prettyjson" && profile_format != 
"summary") {
Done in PS7. Summary output now reads the query id from the profile root and 
only falls back to log metadata when needed. Added WebUI thrift summary 
coverage for this path.


http://gerrit.cloudera.org:8080/#/c/24353/7/be/src/util/summary-util-test.cc
File be/src/util/summary-util-test.cc:

http://gerrit.cloudera.org:8080/#/c/24353/7/be/src/util/summary-util-test.cc@115
PS7, Line 115:   // Keep the percentage width-aligned with the Web UI progress 
formatter.
Done in PS7. Added a short comment explaining that the extra spacing keeps the 
percentage width-aligned with the Web UI formatter.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I35da958106ffb35b14ef867da50ed23ffda0a45b
Gerrit-Change-Number: 24353
Gerrit-PatchSet: 7
Gerrit-Owner: Aleksandr Efimov <[email protected]>
Gerrit-Reviewer: Aleksandr Efimov <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Jason Fehr <[email protected]>
Gerrit-Reviewer: Quanlong Huang <[email protected]>
Gerrit-Comment-Date: Tue, 09 Jun 2026 12:11:11 +0000
Gerrit-HasComments: Yes

Reply via email to