You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Jason Fehr (Code Review)" <ge...@cloudera.org> on 2025/03/24 17:20:40 UTC
[Impala-ASF-CR] IMPALA-13885: Add Query Id to Workload Management Insert Logs
Jason Fehr has uploaded this change for review. ( http://gerrit.cloudera.org:8080/22656
Change subject: IMPALA-13885: Add Query Id to Workload Management Insert Logs
......................................................................
IMPALA-13885: Add Query Id to Workload Management Insert Logs
When the workload management code inserts rows into the completed
queries table, it logs an entry with information on that insert but
does not include the query id in the log line. This lack of query id
causes extra steps to trace cases where the insert DML failed.
This change adds the query id to both the success and failure log
messages logged by the workload management main processing loop. It
also adds the error message to the failure log messages.
Additional minor cleanup was done to provide error messages on python
custom cluster test asserts, add additional asserts for the updated
log messages, and restore a trailing newline in the workload
management startup flags definition file.
All test_query_log.py tests passed locally. These tests were the only
tests that asserted the log messages that were modified.
Change-Id: I3c0816f9eb6bac8c891fd0e249de8863115bf466
---
M be/src/service/workload-management-worker.cc
M be/src/workload_mgmt/workload-management-flags.cc
M tests/custom_cluster/test_query_log.py
3 files changed, 48 insertions(+), 25 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/56/22656/4
--
To view, visit http://gerrit.cloudera.org:8080/22656
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I3c0816f9eb6bac8c891fd0e249de8863115bf466
Gerrit-Change-Number: 22656
Gerrit-PatchSet: 4
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
[Impala-ASF-CR] IMPALA-13885: Add Query Id to Workload Management Insert Logs
Posted by "Jason Fehr (Code Review)" <ge...@cloudera.org>.
Hello Riza Suminto, Abhishek Rawat, gaurav singh, Michael Smith, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/22656
to look at the new patch set (#7).
Change subject: IMPALA-13885: Add Query Id to Workload Management Insert Logs
......................................................................
IMPALA-13885: Add Query Id to Workload Management Insert Logs
When the workload management code inserts rows into the completed
queries table, it logs an entry with information on that insert but
does not include the query id in the log line. This lack of query id
causes extra steps to trace cases where the insert DML failed.
This change adds the query id to both the success and failure log
messages logged by the workload management main processing loop. It
also adds the error message to the failure log messages.
Additional minor cleanup was done to provide error messages on python
custom cluster test asserts, add additional asserts for the updated
log messages, and restore a trailing newline in the workload
management startup flags definition file.
All test_query_log.py tests passed locally. These tests were the only
tests that asserted the log messages that were modified.
Change-Id: I3c0816f9eb6bac8c891fd0e249de8863115bf466
---
M be/src/service/workload-management-worker.cc
M be/src/workload_mgmt/workload-management-flags.cc
M tests/custom_cluster/test_query_log.py
3 files changed, 55 insertions(+), 25 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/56/22656/7
--
To view, visit http://gerrit.cloudera.org:8080/22656
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3c0816f9eb6bac8c891fd0e249de8863115bf466
Gerrit-Change-Number: 22656
Gerrit-PatchSet: 7
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: gaurav singh <gs...@cloudera.com>
[Impala-ASF-CR] IMPALA-13885: Add Query Id to Workload Management Insert Logs
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22656 )
Change subject: IMPALA-13885: Add Query Id to Workload Management Insert Logs
......................................................................
Patch Set 8:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/18694/ : 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/22656
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3c0816f9eb6bac8c891fd0e249de8863115bf466
Gerrit-Change-Number: 22656
Gerrit-PatchSet: 8
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: gaurav singh <gs...@cloudera.com>
Gerrit-Comment-Date: Tue, 25 Mar 2025 23:12:11 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13885: Add Query Id to Workload Management Insert Logs
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22656 )
Change subject: IMPALA-13885: Add Query Id to Workload Management Insert Logs
......................................................................
Patch Set 7:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/18679/ : 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/22656
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3c0816f9eb6bac8c891fd0e249de8863115bf466
Gerrit-Change-Number: 22656
Gerrit-PatchSet: 7
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: gaurav singh <gs...@cloudera.com>
Gerrit-Comment-Date: Mon, 24 Mar 2025 23:33:10 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13885: Add Query Id to Workload Management Insert Logs
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22656 )
Change subject: IMPALA-13885: Add Query Id to Workload Management Insert Logs
......................................................................
Patch Set 4:
(2 comments)
gerrit-auto-critic failed. You can reproduce it locally using command:
python3 bin/jenkins/critique-gerrit-review.py --dryrun
To run it, you might need a virtual env with Python3's venv installed.
http://gerrit.cloudera.org:8080/#/c/22656/4/tests/custom_cluster/test_query_log.py
File tests/custom_cluster/test_query_log.py:
http://gerrit.cloudera.org:8080/#/c/22656/4/tests/custom_cluster/test_query_log.py@717
PS4, Line 717: ,
flake8: E231 missing whitespace after ','
http://gerrit.cloudera.org:8080/#/c/22656/4/tests/custom_cluster/test_query_log.py@829
PS4, Line 829: ,
flake8: E231 missing whitespace after ','
--
To view, visit http://gerrit.cloudera.org:8080/22656
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3c0816f9eb6bac8c891fd0e249de8863115bf466
Gerrit-Change-Number: 22656
Gerrit-PatchSet: 4
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Mon, 24 Mar 2025 17:22:53 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13885: Add Query Id to Workload Management Insert Logs
Posted by "Jason Fehr (Code Review)" <ge...@cloudera.org>.
Hello Riza Suminto, Abhishek Rawat, gaurav singh, Michael Smith, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/22656
to look at the new patch set (#8).
Change subject: IMPALA-13885: Add Query Id to Workload Management Insert Logs
......................................................................
IMPALA-13885: Add Query Id to Workload Management Insert Logs
When the workload management code inserts rows into the completed
queries table, it logs an entry with information on that insert but
does not include the query id in the log line. This lack of query id
causes extra steps to trace cases where the insert DML failed.
This change adds the query id to both the success and failure log
messages logged by the workload management main processing loop. It
also adds the error message to the failure log messages.
Additional minor cleanup was done to provide error messages on python
custom cluster test asserts, add additional asserts for the updated
log messages, and restore a trailing newline in the workload
management startup flags definition file.
All test_query_log.py tests passed locally. These tests were the only
tests that asserted the log messages that were modified.
Change-Id: I3c0816f9eb6bac8c891fd0e249de8863115bf466
---
M be/src/service/workload-management-worker.cc
M be/src/workload_mgmt/workload-management-flags.cc
M tests/custom_cluster/test_query_log.py
3 files changed, 57 insertions(+), 25 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/56/22656/8
--
To view, visit http://gerrit.cloudera.org:8080/22656
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3c0816f9eb6bac8c891fd0e249de8863115bf466
Gerrit-Change-Number: 22656
Gerrit-PatchSet: 8
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: gaurav singh <gs...@cloudera.com>
[Impala-ASF-CR] IMPALA-13885: Add Query Id to Workload Management Insert Logs
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22656 )
Change subject: IMPALA-13885: Add Query Id to Workload Management Insert Logs
......................................................................
Patch Set 9:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/11648/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/22656
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3c0816f9eb6bac8c891fd0e249de8863115bf466
Gerrit-Change-Number: 22656
Gerrit-PatchSet: 9
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: gaurav singh <gs...@cloudera.com>
Gerrit-Comment-Date: Tue, 25 Mar 2025 22:56:47 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13885: Add Query Id to Workload Management Insert Logs
Posted by "Jason Fehr (Code Review)" <ge...@cloudera.org>.
Jason Fehr has uploaded a new patch set (#6). ( http://gerrit.cloudera.org:8080/22656 )
Change subject: IMPALA-13885: Add Query Id to Workload Management Insert Logs
......................................................................
IMPALA-13885: Add Query Id to Workload Management Insert Logs
When the workload management code inserts rows into the completed
queries table, it logs an entry with information on that insert but
does not include the query id in the log line. This lack of query id
causes extra steps to trace cases where the insert DML failed.
This change adds the query id to both the success and failure log
messages logged by the workload management main processing loop. It
also adds the error message to the failure log messages.
Additional minor cleanup was done to provide error messages on python
custom cluster test asserts, add additional asserts for the updated
log messages, and restore a trailing newline in the workload
management startup flags definition file.
All test_query_log.py tests passed locally. These tests were the only
tests that asserted the log messages that were modified.
Change-Id: I3c0816f9eb6bac8c891fd0e249de8863115bf466
---
M be/src/service/workload-management-worker.cc
M be/src/workload_mgmt/workload-management-flags.cc
M tests/custom_cluster/test_query_log.py
3 files changed, 50 insertions(+), 25 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/56/22656/6
--
To view, visit http://gerrit.cloudera.org:8080/22656
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3c0816f9eb6bac8c891fd0e249de8863115bf466
Gerrit-Change-Number: 22656
Gerrit-PatchSet: 6
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
[Impala-ASF-CR] IMPALA-13885: Add Query Id to Workload Management Insert Logs
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22656 )
Change subject: IMPALA-13885: Add Query Id to Workload Management Insert Logs
......................................................................
Patch Set 9: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/22656
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3c0816f9eb6bac8c891fd0e249de8863115bf466
Gerrit-Change-Number: 22656
Gerrit-PatchSet: 9
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: gaurav singh <gs...@cloudera.com>
Gerrit-Comment-Date: Tue, 25 Mar 2025 22:56:46 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13885: Add Query Id to Workload Management Insert Logs
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/22656 )
Change subject: IMPALA-13885: Add Query Id to Workload Management Insert Logs
......................................................................
Patch Set 7:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/22656/6/tests/custom_cluster/test_query_log.py
File tests/custom_cluster/test_query_log.py:
http://gerrit.cloudera.org:8080/#/c/22656/6/tests/custom_cluster/test_query_log.py@583
PS6, Line 583: # When a workload management insert DML fails, a warning message is logged that
: # contains information about the DML. These two assertions ensure the message is
: # logged and important fields are correct.
: res = self.assert_impalad_log_contains(
: level="WARNING",
: line_regex=r'failed to write completed queries table="{}" record_count=1 '
: r'bytes=\S+\s\S+ gather_time=\S+
> I could not get placeholders to work on multi-line strings. I now realize
Ack
http://gerrit.cloudera.org:8080/#/c/22656/7/tests/custom_cluster/test_query_log.py
File tests/custom_cluster/test_query_log.py:
http://gerrit.cloudera.org:8080/#/c/22656/7/tests/custom_cluster/test_query_log.py@588
PS7, Line 588: record_count=1
record_count can be validated separately just like msg.
--
To view, visit http://gerrit.cloudera.org:8080/22656
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3c0816f9eb6bac8c891fd0e249de8863115bf466
Gerrit-Change-Number: 22656
Gerrit-PatchSet: 7
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: gaurav singh <gs...@cloudera.com>
Gerrit-Comment-Date: Mon, 24 Mar 2025 23:23:15 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13885: Add Query Id to Workload Management Insert Logs
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22656 )
Change subject: IMPALA-13885: Add Query Id to Workload Management Insert Logs
......................................................................
Patch Set 4:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/18672/ : 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/22656
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3c0816f9eb6bac8c891fd0e249de8863115bf466
Gerrit-Change-Number: 22656
Gerrit-PatchSet: 4
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Comment-Date: Mon, 24 Mar 2025 17:45:41 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13885: Add Query Id to Workload Management Insert Logs
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22656 )
Change subject: IMPALA-13885: Add Query Id to Workload Management Insert Logs
......................................................................
Patch Set 9: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/22656
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3c0816f9eb6bac8c891fd0e249de8863115bf466
Gerrit-Change-Number: 22656
Gerrit-PatchSet: 9
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: gaurav singh <gs...@cloudera.com>
Gerrit-Comment-Date: Wed, 26 Mar 2025 04:36:35 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13885: Add Query Id to Workload Management Insert Logs
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/22656 )
Change subject: IMPALA-13885: Add Query Id to Workload Management Insert Logs
......................................................................
Patch Set 8: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/22656
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3c0816f9eb6bac8c891fd0e249de8863115bf466
Gerrit-Change-Number: 22656
Gerrit-PatchSet: 8
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: gaurav singh <gs...@cloudera.com>
Gerrit-Comment-Date: Tue, 25 Mar 2025 22:50:08 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13885: Add Query Id to Workload Management Insert Logs
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22656 )
Change subject: IMPALA-13885: Add Query Id to Workload Management Insert Logs
......................................................................
Patch Set 6:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/18673/ : 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/22656
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3c0816f9eb6bac8c891fd0e249de8863115bf466
Gerrit-Change-Number: 22656
Gerrit-PatchSet: 6
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: gaurav singh <gs...@cloudera.com>
Gerrit-Comment-Date: Mon, 24 Mar 2025 18:06:39 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13885: Add Query Id to Workload Management Insert Logs
Posted by "Jason Fehr (Code Review)" <ge...@cloudera.org>.
Jason Fehr has posted comments on this change. ( http://gerrit.cloudera.org:8080/22656 )
Change subject: IMPALA-13885: Add Query Id to Workload Management Insert Logs
......................................................................
Patch Set 6:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/22656/6/be/src/workload_mgmt/workload-management-flags.cc
File be/src/workload_mgmt/workload-management-flags.cc:
http://gerrit.cloudera.org:8080/#/c/22656/6/be/src/workload_mgmt/workload-management-flags.cc@200
PS6, Line 200: });
Change https://gerrit.cloudera.org/c/22652/ accidentally removed the ending newline of this file. This change is adding the newline back.
--
To view, visit http://gerrit.cloudera.org:8080/22656
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3c0816f9eb6bac8c891fd0e249de8863115bf466
Gerrit-Change-Number: 22656
Gerrit-PatchSet: 6
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Comment-Date: Mon, 24 Mar 2025 17:39:20 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13885: Add Query Id to Workload Management Insert Logs
Posted by "Jason Fehr (Code Review)" <ge...@cloudera.org>.
Jason Fehr has posted comments on this change. ( http://gerrit.cloudera.org:8080/22656 )
Change subject: IMPALA-13885: Add Query Id to Workload Management Insert Logs
......................................................................
Patch Set 6:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/22656/6/tests/custom_cluster/test_query_log.py
File tests/custom_cluster/test_query_log.py:
http://gerrit.cloudera.org:8080/#/c/22656/6/tests/custom_cluster/test_query_log.py@583
PS6, Line 583: line_regex=r'failed to write completed queries '
: + r'table="' + QUERY_TBL_LOG + r'" record_count=\d+ '
: + r'bytes=\S+\s\S+ gather_time=\S+ exec_time=\S+ '
: + r'query_id="' + self.insert_query_id
: + r'" msg="Query ' + self.insert_query_id
: + r' expired due to execution time limit of'
: + r' 1s000ms"',
> Can craft the string pattern first with .format().
I could not get placeholders to work on multi-line strings. I now realize that is because the .format() was only being applied to the very last substring because I joined the substrings with + which I should not have done.
I would rather not clutter up the code with example log lines. Searching the Impala code base for the string "failed to write completed queries" turns up a single match, thus it should be fairly quick to understand the log message being asserted.
The other fields are variable and don't have a source of truth that could be used in the assertions. While I would like to match on some of them, I would have to use ranges and the test would probably end up being flaky resulting in the ranges being modified.
I like that idea to validate the msg parameter separately. Done.
--
To view, visit http://gerrit.cloudera.org:8080/22656
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3c0816f9eb6bac8c891fd0e249de8863115bf466
Gerrit-Change-Number: 22656
Gerrit-PatchSet: 6
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: gaurav singh <gs...@cloudera.com>
Gerrit-Comment-Date: Mon, 24 Mar 2025 23:06:53 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13885: Add Query Id to Workload Management Insert Logs
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/22656 )
Change subject: IMPALA-13885: Add Query Id to Workload Management Insert Logs
......................................................................
Patch Set 6:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/22656/6/tests/custom_cluster/test_query_log.py
File tests/custom_cluster/test_query_log.py:
http://gerrit.cloudera.org:8080/#/c/22656/6/tests/custom_cluster/test_query_log.py@583
PS6, Line 583: line_regex=r'failed to write completed queries '
: + r'table="' + QUERY_TBL_LOG + r'" record_count=\d+ '
: + r'bytes=\S+\s\S+ gather_time=\S+ exec_time=\S+ '
: + r'query_id="' + self.insert_query_id
: + r'" msg="Query ' + self.insert_query_id
: + r' expired due to execution time limit of'
: + r' 1s000ms"',
Can craft the string pattern first with .format().
Please add comment for example of log lines to match.
Is it important to match all the details in this test (fields other than query_id)?
This assert can fail without clear explanation if query_id is there, but msg differ from expectation.
Can the msg validated from the returned value of assert_impalad_log_contains?
--
To view, visit http://gerrit.cloudera.org:8080/22656
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3c0816f9eb6bac8c891fd0e249de8863115bf466
Gerrit-Change-Number: 22656
Gerrit-PatchSet: 6
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: gaurav singh <gs...@cloudera.com>
Gerrit-Comment-Date: Mon, 24 Mar 2025 19:56:17 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13885: Add Query Id to Workload Management Insert Logs
Posted by "Jason Fehr (Code Review)" <ge...@cloudera.org>.
Jason Fehr has posted comments on this change. ( http://gerrit.cloudera.org:8080/22656 )
Change subject: IMPALA-13885: Add Query Id to Workload Management Insert Logs
......................................................................
Patch Set 8:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/22656/7/tests/custom_cluster/test_query_log.py
File tests/custom_cluster/test_query_log.py:
http://gerrit.cloudera.org:8080/#/c/22656/7/tests/custom_cluster/test_query_log.py@588
PS7, Line 588: record_count=(
> record_count can be validated separately just like msg.
Done
--
To view, visit http://gerrit.cloudera.org:8080/22656
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3c0816f9eb6bac8c891fd0e249de8863115bf466
Gerrit-Change-Number: 22656
Gerrit-PatchSet: 8
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: gaurav singh <gs...@cloudera.com>
Gerrit-Comment-Date: Tue, 25 Mar 2025 22:45:28 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13885: Add Query Id to Workload Management Insert Logs
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/22656 )
Change subject: IMPALA-13885: Add Query Id to Workload Management Insert Logs
......................................................................
IMPALA-13885: Add Query Id to Workload Management Insert Logs
When the workload management code inserts rows into the completed
queries table, it logs an entry with information on that insert but
does not include the query id in the log line. This lack of query id
causes extra steps to trace cases where the insert DML failed.
This change adds the query id to both the success and failure log
messages logged by the workload management main processing loop. It
also adds the error message to the failure log messages.
Additional minor cleanup was done to provide error messages on python
custom cluster test asserts, add additional asserts for the updated
log messages, and restore a trailing newline in the workload
management startup flags definition file.
All test_query_log.py tests passed locally. These tests were the only
tests that asserted the log messages that were modified.
Change-Id: I3c0816f9eb6bac8c891fd0e249de8863115bf466
Reviewed-on: http://gerrit.cloudera.org:8080/22656
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/service/workload-management-worker.cc
M be/src/workload_mgmt/workload-management-flags.cc
M tests/custom_cluster/test_query_log.py
3 files changed, 57 insertions(+), 25 deletions(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/22656
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I3c0816f9eb6bac8c891fd0e249de8863115bf466
Gerrit-Change-Number: 22656
Gerrit-PatchSet: 10
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: gaurav singh <gs...@cloudera.com>