Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/22437 )
Change subject: IMPALA-13033: support parsing thrift profiles downloaded from WebUI ...................................................................... Patch Set 2: (6 comments) http://gerrit.cloudera.org:8080/#/c/22437/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/22437/2//COMMIT_MSG@7 PS2, Line 7: Redundant whitespaces. Please remove. http://gerrit.cloudera.org:8080/#/c/22437/2/be/src/util/impala-profile-tool.cc File be/src/util/impala-profile-tool.cc: http://gerrit.cloudera.org:8080/#/c/22437/2/be/src/util/impala-profile-tool.cc@111 PS2, Line 111: istringstream retry_liness(line); Unused variable? http://gerrit.cloudera.org:8080/#/c/22437/2/be/src/util/impala-profile-tool.cc@112 PS2, Line 112: // if parsing fails, parse the entire line as the encoded_profile without a timestamp or query_id for thrift format. > line too long (116 > 90) Indentation is also wrong. Please fix. http://gerrit.cloudera.org:8080/#/c/22437/2/be/src/util/impala-profile-tool.cc@113 PS2, Line 113: encoded_profile = line; What is the right init value for 'timestamp' and 'query_id' in this case? http://gerrit.cloudera.org:8080/#/c/22437/2/be/src/util/impala-profile-tool.cc@114 PS2, Line 114: cerr << "Error parsing line " << lineno << ": '" << line << "'\n"; Error message should mention that the same like is retried as if the entire line is the serialized thrift. http://gerrit.cloudera.org:8080/#/c/22437/2/be/src/util/impala-profile-tool.cc@115 PS2, Line 115: ++errors; : continue; Should not increase errors and continue if attempt to decode the whole line as is. -- To view, visit http://gerrit.cloudera.org:8080/22437 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iced6dd189aebd0f61ae6cb1bfac7f9dae31c6a4e Gerrit-Change-Number: 22437 Gerrit-PatchSet: 2 Gerrit-Owner: Anshula Jain <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Riza Suminto <[email protected]> Gerrit-Comment-Date: Wed, 05 Feb 2025 19:01:39 +0000 Gerrit-HasComments: Yes
