Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/20201 )
Change subject: IMPALA-12278: Adding query option max_error_message_length ...................................................................... Patch Set 8: (8 comments) http://gerrit.cloudera.org:8080/#/c/20201/8/be/src/runtime/runtime-state.cc File be/src/runtime/runtime-state.cc: http://gerrit.cloudera.org:8080/#/c/20201/8/be/src/runtime/runtime-state.cc@208 PS8, Line 208: message.msg() I thik we should truncate this as well. We can use the new query option or add a startup flag similar to max_error_logs_per_instance. http://gerrit.cloudera.org:8080/#/c/20201/8/be/src/runtime/runtime-state.cc@225 PS8, Line 225: error content beyond limit nit: "error message exceeds the limit" http://gerrit.cloudera.org:8080/#/c/20201/8/be/src/util/error-util.cc File be/src/util/error-util.cc: http://gerrit.cloudera.org:8080/#/c/20201/8/be/src/util/error-util.cc@235 PS8, Line 235: DCHECK_OK(status); What about 'details_' ? Should we also trancate the total length of strings in it? http://gerrit.cloudera.org:8080/#/c/20201/8/be/src/util/error-util.cc@261 PS8, Line 261: ErrorMsg GetTruncatedErrorMessage(const ErrorMsg& e, int max_error_message_length) { Passing the result as a pointer parameter can save some string copies void GetTruncatedErrorMessage(const ErrorMsg& e, int max_error_message_length, ErrorMsg* result) http://gerrit.cloudera.org:8080/#/c/20201/8/common/thrift/ImpalaService.thrift File common/thrift/ImpalaService.thrift: http://gerrit.cloudera.org:8080/#/c/20201/8/common/thrift/ImpalaService.thrift@837 PS8, Line 837: ; nit: Use "." BTW, is it the limit on the aggregated error message? Or is it a limit on each error reported by each executor? http://gerrit.cloudera.org:8080/#/c/20201/8/tests/custom_cluster/test_logging.py File tests/custom_cluster/test_logging.py: http://gerrit.cloudera.org:8080/#/c/20201/8/tests/custom_cluster/test_logging.py@65 PS8, Line 65: def test_max_error_message_length(self): This doesn't need customized startup flags. We can add it in a non custom-cluster test, e.g. tests/query_test/test_errorlog.py. http://gerrit.cloudera.org:8080/#/c/20201/8/tests/custom_cluster/test_logging.py@76 PS8, Line 76: assert re.search("UDF WARNING: Failed to parse json at position 0 since: " nit: can be "UDF WARNING: Failed to parse json at position 0 since: Invalid value.. Json string:\njust_\n" in profile http://gerrit.cloudera.org:8080/#/c/20201/8/tests/custom_cluster/test_logging.py@81 PS8, Line 81: assert not re.search("UDF WARNING:", profile) nit: can be assert "UDF WARNING:" not in profile -- To view, visit http://gerrit.cloudera.org:8080/20201 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I22bd1fe8b15ab83fe5f6fd4d5455c5f68e247ba6 Gerrit-Change-Number: 20201 Gerrit-PatchSet: 8 Gerrit-Owner: ttttttz <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: ttttttz <[email protected]> Gerrit-Comment-Date: Mon, 14 Aug 2023 02:03:56 +0000 Gerrit-HasComments: Yes
