[Impala-ASF-CR] IMPALA-5845: Limit number of non-fatal error logging to INFO
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/18565 ) Change subject: IMPALA-5845: Limit number of non-fatal error logging to INFO .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/18565/3/tests/custom_cluster/test_breakpad.py File tests/custom_cluster/test_breakpad.py: http://gerrit.cloudera.org:8080/#/c/18565/3/tests/custom_cluster/test_breakpad.py@499 PS3, Line 499: TestBreakpadBase Is there a reason why this goes to test_breakpad.py ? Couldn't it be a simple CustomClusterTest? -- To view, visit http://gerrit.cloudera.org:8080/18565 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I924768ec461735c172fbf75d6415033bbdb77f9b Gerrit-Change-Number: 18565 Gerrit-PatchSet: 1 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Wed, 01 Jun 2022 10:10:29 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5845: Limit number of non-fatal error logging to INFO
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/18565 ) Change subject: IMPALA-5845: Limit number of non-fatal error logging to INFO .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/18565/3/be/src/runtime/runtime-state.h File be/src/runtime/runtime-state.h: http://gerrit.cloudera.org:8080/#/c/18565/3/be/src/runtime/runtime-state.h@175 PS3, Line 175: return query_options().max_errors <= 0 ? 100 : query_options().max_errors; > Why is 100 used here if the default is 2000? Seems there should be only 1 d This code here is a refactoring of similarly deleted code in query-state.cc. The reason to have this is to not override it early in query-state.cc. Thus, we are keeping the actual option supplied by user and ignore max_error_logs_per_instance flag if max_errors < 0. max_error_logs_per_instance is a separate threshold, implemented as backend flag. We pick 2000 as a double of max_erros per documentation. https://impala.apache.org/docs/build/html/topics/impala_max_errors.html However, this turns out to be a documentation bug: https://issues.apache.org/jira/browse/IMPALA-11328 -- To view, visit http://gerrit.cloudera.org:8080/18565 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I924768ec461735c172fbf75d6415033bbdb77f9b Gerrit-Change-Number: 18565 Gerrit-PatchSet: 3 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Wed, 01 Jun 2022 03:41:25 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5845: Limit number of non-fatal error logging to INFO
Kurt Deschler has posted comments on this change. ( http://gerrit.cloudera.org:8080/18565 ) Change subject: IMPALA-5845: Limit number of non-fatal error logging to INFO .. Patch Set 3: (6 comments) http://gerrit.cloudera.org:8080/#/c/18565/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/18565/3//COMMIT_MSG@9 PS3, Line 9: RuntimeState::LogError() do both of error aggregation to coordinator and ..does both error.. http://gerrit.cloudera.org:8080/#/c/18565/3//COMMIT_MSG@12 PS3, Line 12: analyze other more significant log lines. This patch limit the number of ..limits the number or errors.. http://gerrit.cloudera.org:8080/#/c/18565/3//COMMIT_MSG@18 PS3, Line 18: set query option max_errors < 0, which in that case all errors ..user sets.. http://gerrit.cloudera.org:8080/#/c/18565/3//COMMIT_MSG@21 PS3, Line 21: This patch also fix bug where error count is not increased for fixes a bug http://gerrit.cloudera.org:8080/#/c/18565/3/be/src/runtime/runtime-state.h File be/src/runtime/runtime-state.h: http://gerrit.cloudera.org:8080/#/c/18565/3/be/src/runtime/runtime-state.h@175 PS3, Line 175: return query_options().max_errors <= 0 ? 100 : query_options().max_errors; Why is 100 used here if the default is 2000? Seems there should be only 1 default. If we want to have a second threshold, that should be a different parameter. http://gerrit.cloudera.org:8080/#/c/18565/3/be/src/runtime/runtime-state.cc File be/src/runtime/runtime-state.cc: http://gerrit.cloudera.org:8080/#/c/18565/3/be/src/runtime/runtime-state.cc@221 PS3, Line 221: // Appending general error is expensive since it append message. Nit: grammar in comments. expensive since it writes the entire message to the log? ..that already exists in error_log_.. -- To view, visit http://gerrit.cloudera.org:8080/18565 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I924768ec461735c172fbf75d6415033bbdb77f9b Gerrit-Change-Number: 18565 Gerrit-PatchSet: 3 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Wed, 01 Jun 2022 02:01:38 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5845: Limit number of non-fatal error logging to INFO
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18565 ) Change subject: IMPALA-5845: Limit number of non-fatal error logging to INFO .. Patch Set 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/10666/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/18565 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I924768ec461735c172fbf75d6415033bbdb77f9b Gerrit-Change-Number: 18565 Gerrit-PatchSet: 3 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Tue, 31 May 2022 22:29:24 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5845: Limit number of non-fatal error logging to INFO
Hello Quanlong Huang, Csaba Ringhofer, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/18565 to look at the new patch set (#3). Change subject: IMPALA-5845: Limit number of non-fatal error logging to INFO .. IMPALA-5845: Limit number of non-fatal error logging to INFO RuntimeState::LogError() do both of error aggregation to coordinator and logging the error to log file depending on the vlog_level. This can flood INFO log if specified vlog_level is 1 and makes it difficult to analyze other more significant log lines. This patch limit the number of error logged to INFO based on max_error_logs_per_instance flag (default is 2000). When this number exceeded, vlog_level=1 will be downgraded to vlog_level=2. To allow easy debugging in the future, this flag will be ignored if user set query option max_errors < 0, which in that case all errors targetting vlog_level 1 will be logged. This patch also fix bug where error count is not increased for non-general error code that is already in 'error_log_' map. Testing: - Add test_breakpad.py::TestLoggingCore Change-Id: I924768ec461735c172fbf75d6415033bbdb77f9b --- M be/src/runtime/query-state.cc M be/src/runtime/runtime-state.cc M be/src/runtime/runtime-state.h M tests/custom_cluster/test_breakpad.py 4 files changed, 82 insertions(+), 11 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/65/18565/3 -- To view, visit http://gerrit.cloudera.org:8080/18565 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I924768ec461735c172fbf75d6415033bbdb77f9b Gerrit-Change-Number: 18565 Gerrit-PatchSet: 3 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Riza Suminto
[Impala-ASF-CR] IMPALA-5845: Limit number of non-fatal error logging to INFO
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18565 ) Change subject: IMPALA-5845: Limit number of non-fatal error logging to INFO .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/10665/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/18565 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I924768ec461735c172fbf75d6415033bbdb77f9b Gerrit-Change-Number: 18565 Gerrit-PatchSet: 2 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Tue, 31 May 2022 22:14:23 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5845: Limit number of non-fatal error logging to INFO
Hello Quanlong Huang, Csaba Ringhofer, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/18565 to look at the new patch set (#2). Change subject: IMPALA-5845: Limit number of non-fatal error logging to INFO .. IMPALA-5845: Limit number of non-fatal error logging to INFO RuntimeState::LogError() do both of error aggregation to coordinator and logging the error to log file depending on the vlog_level. This can flood INFO log if specified vlog_level is 1 and makes it difficult to analyze other more significant log lines. This patch limit the number of error logged to INFO based on max_error_logs_per_instance flag (default is 2000). When this number exceeded, vlog_level=1 will be downgraded to vlog_level=2. To allow easy debugging in the future, this flag will be ignored if user set query option max_errors < 0, which in that case all errors targetting vlog_level 1 will be logged. Testing: - Add test_breakpad.py::TestLoggingCore Change-Id: I924768ec461735c172fbf75d6415033bbdb77f9b --- M be/src/runtime/query-state.cc M be/src/runtime/runtime-state.cc M be/src/runtime/runtime-state.h M tests/custom_cluster/test_breakpad.py 4 files changed, 79 insertions(+), 12 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/65/18565/2 -- To view, visit http://gerrit.cloudera.org:8080/18565 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I924768ec461735c172fbf75d6415033bbdb77f9b Gerrit-Change-Number: 18565 Gerrit-PatchSet: 2 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Riza Suminto
[Impala-ASF-CR] IMPALA-5845: Limit number of non-fatal error logging to INFO
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18565 ) Change subject: IMPALA-5845: Limit number of non-fatal error logging to INFO .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/10644/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/18565 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I924768ec461735c172fbf75d6415033bbdb77f9b Gerrit-Change-Number: 18565 Gerrit-PatchSet: 1 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Thu, 26 May 2022 07:43:38 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5845: Limit number of non-fatal error logging to INFO
Riza Suminto has uploaded this change for review. ( http://gerrit.cloudera.org:8080/18565 Change subject: IMPALA-5845: Limit number of non-fatal error logging to INFO .. IMPALA-5845: Limit number of non-fatal error logging to INFO RuntimeState::LogError() do both of error aggregation to coordinator and logging the error to log file depending on the vlog_level. This can flood INFO log if specified vlog_level is 1 and makes it difficult to analyze other more significant log lines. This patch limit the number of error logged to INFO based on MAX_ERROR query option (default is 1000). When this number exceeded, vlog_level=1 will be downgraded to vlog_level=2. Testing: - Add test_breakpad.py::TestLoggingCore::test_max_errors Change-Id: I924768ec461735c172fbf75d6415033bbdb77f9b --- M be/src/runtime/runtime-state.cc M be/src/runtime/runtime-state.h M tests/custom_cluster/test_breakpad.py 3 files changed, 42 insertions(+), 7 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/65/18565/1 -- To view, visit http://gerrit.cloudera.org:8080/18565 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I924768ec461735c172fbf75d6415033bbdb77f9b Gerrit-Change-Number: 18565 Gerrit-PatchSet: 1 Gerrit-Owner: Riza Suminto
