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 2024/03/15 22:41:07 UTC
[Impala-ASF-CR] IMPALA-12913: Refactor Workload Management Custom Cluser Tests
Jason Fehr has uploaded this change for review. ( http://gerrit.cloudera.org:8080/21153
Change subject: IMPALA-12913: Refactor Workload Management Custom Cluser Tests
......................................................................
IMPALA-12913: Refactor Workload Management Custom Cluser Tests
The custom cluster tests that assert the workload
management functionality to insert completed queries into
the impala_query_log table were inefficient because they
created their own database tables and added data to those
tables.
This patch updates these tests to use the existing tables
in the functional database where possible. The few tests
that need their own tables now have those tables set up in
a database created by the pytest unique_database fixture
instead of using the default database.
A new table has also been added to the functional database.
This table is named zipcode_timezones and contains two
columns, the first having a few zipcodes and the second
having their corresponding timezone. This table can be used
to join the zipcode_incomes and alltimezones tables. This
table is populated by a new csv file in the testdata
directory.
Change-Id: I1e3249a8f306cf43de0d6f6586711c779399e83b
---
A testdata/data/zipcodes_timezones.csv
M testdata/datasets/functional/functional_schema_template.sql
M tests/custom_cluster/test_query_log.py
3 files changed, 662 insertions(+), 194 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/53/21153/1
--
To view, visit http://gerrit.cloudera.org:8080/21153
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I1e3249a8f306cf43de0d6f6586711c779399e83b
Gerrit-Change-Number: 21153
Gerrit-PatchSet: 1
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
[Impala-ASF-CR] IMPALA-12913: Refactor Workload Management Custom Cluster Tests
Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21153 )
Change subject: IMPALA-12913: Refactor Workload Management Custom Cluster Tests
......................................................................
Patch Set 7:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/21153/6/tests/custom_cluster/test_query_log.py
File tests/custom_cluster/test_query_log.py:
http://gerrit.cloudera.org:8080/#/c/21153/6/tests/custom_cluster/test_query_log.py@51
PS6, Line 51: cls.PROTOCOL_BEESWAX, cls.PROTOCOL_HS2))
I wouldn't expect any difference in behavior between Beeswax and HS2, from the frontend these are very standard queries. Was there a specific reason to do both?
http://gerrit.cloudera.org:8080/#/c/21153/6/tests/custom_cluster/test_query_log.py@97
PS6, Line 97: handle = client.execute_async("select '{0}'".format(rand_long_str))
Could you comment that we use async to avoid fetching the large result string.
http://gerrit.cloudera.org:8080/#/c/21153/6/tests/custom_cluster/test_query_log.py@383
PS6, Line 383: def test_query_log_table_create_table_sys_db_blocked(self, vector):
This isn't specific to query_log, could we move the sys db tests to a new file?
http://gerrit.cloudera.org:8080/#/c/21153/7/tests/custom_cluster/test_query_log.py
File tests/custom_cluster/test_query_log.py:
http://gerrit.cloudera.org:8080/#/c/21153/7/tests/custom_cluster/test_query_log.py@208
PS7, Line 208: sleep(1)
Why do you sleep between runs?
http://gerrit.cloudera.org:8080/#/c/21153/7/tests/custom_cluster/test_query_log.py@214
PS7, Line 214: sleep(10)
What are you actually waiting on. Is there a data-cache metric we can watch for this?
--
To view, visit http://gerrit.cloudera.org:8080/21153
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e3249a8f306cf43de0d6f6586711c779399e83b
Gerrit-Change-Number: 21153
Gerrit-PatchSet: 7
Gerrit-Owner: Jason Fehr <jf...@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-Comment-Date: Tue, 26 Mar 2024 16:58:02 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12913: Refactor Workload Management Custom Cluster Tests
Posted by "Jason Fehr (Code Review)" <ge...@cloudera.org>.
Jason Fehr has posted comments on this change. ( http://gerrit.cloudera.org:8080/21153 )
Change subject: IMPALA-12913: Refactor Workload Management Custom Cluster Tests
......................................................................
Patch Set 7:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/21153/6/tests/custom_cluster/test_query_log.py
File tests/custom_cluster/test_query_log.py:
http://gerrit.cloudera.org:8080/#/c/21153/6/tests/custom_cluster/test_query_log.py@881
PS6, Line 881: TestQueryLogTableBas
> This should be TestQueryLogTableBase. Otherwise, it inherits and runs all t
Done
--
To view, visit http://gerrit.cloudera.org:8080/21153
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e3249a8f306cf43de0d6f6586711c779399e83b
Gerrit-Change-Number: 21153
Gerrit-PatchSet: 7
Gerrit-Owner: Jason Fehr <jf...@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-Comment-Date: Tue, 26 Mar 2024 15:32:09 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12913: Refactor Workload Management Custom Cluser Tests
Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21153 )
Change subject: IMPALA-12913: Refactor Workload Management Custom Cluser Tests
......................................................................
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/21153/1/testdata/datasets/functional/functional_schema_template.sql
File testdata/datasets/functional/functional_schema_template.sql:
http://gerrit.cloudera.org:8080/#/c/21153/1/testdata/datasets/functional/functional_schema_template.sql@1804
PS1, Line 1804: CREATE EXTERNAL TABLE IF NOT EXISTS {db_name}{db_suffix}.{table_name} (
There seem to be support for all this above with COLUMNS, ROW_FORMAT, DEPENDENT_LOAD. Why use CREATE instead?
http://gerrit.cloudera.org:8080/#/c/21153/1/tests/custom_cluster/test_query_log.py
File tests/custom_cluster/test_query_log.py:
http://gerrit.cloudera.org:8080/#/c/21153/1/tests/custom_cluster/test_query_log.py@438
PS1, Line 438: client = self.create_impala_client(protocol=vector.get_value('protocol'))
Why do you create a client?
I guess there are clients already set up, but selecting the correct one isn't easy (unless using "run_test_case"). Might be useful to add a test helper to get the existing client for a protocol instead of creating new ones.
--
To view, visit http://gerrit.cloudera.org:8080/21153
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e3249a8f306cf43de0d6f6586711c779399e83b
Gerrit-Change-Number: 21153
Gerrit-PatchSet: 1
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Comment-Date: Fri, 15 Mar 2024 23:49:02 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12913: Refactor Workload Management Custom Cluster Tests
Posted by "Jason Fehr (Code Review)" <ge...@cloudera.org>.
Jason Fehr has posted comments on this change. ( http://gerrit.cloudera.org:8080/21153 )
Change subject: IMPALA-12913: Refactor Workload Management Custom Cluster Tests
......................................................................
Patch Set 5:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/21153/1/tests/custom_cluster/test_query_log.py
File tests/custom_cluster/test_query_log.py:
http://gerrit.cloudera.org:8080/#/c/21153/1/tests/custom_cluster/test_query_log.py@635
PS1, Line 635: catalogd_args="--enable_workload_m
> https://impala.apache.org/docs/build/html/topics/impala_spool_query_results
Based on the comments in IMPALA-12932 that SPOOL_QUERY_RESULTS should be true and the docs need changing, I am deleting this line.
--
To view, visit http://gerrit.cloudera.org:8080/21153
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e3249a8f306cf43de0d6f6586711c779399e83b
Gerrit-Change-Number: 21153
Gerrit-PatchSet: 5
Gerrit-Owner: Jason Fehr <jf...@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-Comment-Date: Thu, 21 Mar 2024 20:27:18 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12913: Refactor Workload Management Custom Cluster Tests
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21153 )
Change subject: IMPALA-12913: Refactor Workload Management Custom Cluster Tests
......................................................................
Patch Set 9:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10434/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/21153
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e3249a8f306cf43de0d6f6586711c779399e83b
Gerrit-Change-Number: 21153
Gerrit-PatchSet: 9
Gerrit-Owner: Jason Fehr <jf...@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-Comment-Date: Tue, 26 Mar 2024 23:34:32 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12913: Refactor Workload Management Custom Cluster Tests
Posted by "Jason Fehr (Code Review)" <ge...@cloudera.org>.
Hello Riza Suminto, Michael Smith, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/21153
to look at the new patch set (#4).
Change subject: IMPALA-12913: Refactor Workload Management Custom Cluster Tests
......................................................................
IMPALA-12913: Refactor Workload Management Custom Cluster Tests
The custom cluster tests that assert the workload
management functionality to insert completed queries into
the impala_query_log table were inefficient because they
created their own database tables and added data to those
tables.
This patch updates these tests to use the existing tables
in the functional database where possible. The few tests
that need their own tables now have those tables set up in
a database created by the pytest unique_database fixture
instead of using the default database.
A new table has also been added to the functional database.
This table is named zipcode_timezones and contains two
columns, the first having a few zipcodes and the second
having their corresponding timezone. This table can be used
to join the zipcode_incomes and alltimezones tables. This
table is populated by a new csv file in the testdata
directory.
Change-Id: I1e3249a8f306cf43de0d6f6586711c779399e83b
---
A testdata/data/zipcodes_timezones.csv
M testdata/datasets/functional/functional_schema_template.sql
M tests/custom_cluster/test_query_log.py
3 files changed, 983 insertions(+), 485 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/53/21153/4
--
To view, visit http://gerrit.cloudera.org:8080/21153
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1e3249a8f306cf43de0d6f6586711c779399e83b
Gerrit-Change-Number: 21153
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-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
[Impala-ASF-CR] IMPALA-12913: Refactor Workload Management Custom Cluster Tests
Posted by "Jason Fehr (Code Review)" <ge...@cloudera.org>.
Hello Riza Suminto, Michael Smith, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/21153
to look at the new patch set (#8).
Change subject: IMPALA-12913: Refactor Workload Management Custom Cluster Tests
......................................................................
IMPALA-12913: Refactor Workload Management Custom Cluster Tests
The custom cluster tests that assert the workload
management functionality to insert completed queries into
the impala_query_log table were inefficient because they
created their own database tables and added data to those
tables.
This patch updates these tests to use the existing tables
in the functional database where possible. The few tests
that need their own tables now have those tables set up in
a database created by the pytest unique_database fixture
instead of using the default database.
A new table has also been added to the functional database.
This table is named zipcode_timezones and contains two
columns, the first having a few zipcodes and the second
having their corresponding timezone. This table can be used
to join the zipcode_incomes and alltimezones tables. This
table is populated by a new csv file in the testdata
directory.
Change-Id: I1e3249a8f306cf43de0d6f6586711c779399e83b
---
A testdata/data/zipcodes_timezones.csv
M testdata/datasets/functional/functional_schema_template.sql
M tests/custom_cluster/test_query_log.py
A tests/custom_cluster/test_sys_db.py
M tests/util/workload_management.py
5 files changed, 1,086 insertions(+), 490 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/53/21153/8
--
To view, visit http://gerrit.cloudera.org:8080/21153
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1e3249a8f306cf43de0d6f6586711c779399e83b
Gerrit-Change-Number: 21153
Gerrit-PatchSet: 8
Gerrit-Owner: Jason Fehr <jf...@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>
[Impala-ASF-CR] IMPALA-12913: Refactor Workload Management Custom Cluster Tests
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21153 )
Change subject: IMPALA-12913: Refactor Workload Management Custom Cluster Tests
......................................................................
Patch Set 3:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/21153/3/tests/custom_cluster/test_query_log.py
File tests/custom_cluster/test_query_log.py:
http://gerrit.cloudera.org:8080/#/c/21153/3/tests/custom_cluster/test_query_log.py@390
PS3, Line 390: r
flake8: F841 local variable 'res' is assigned to but never used
http://gerrit.cloudera.org:8080/#/c/21153/3/tests/custom_cluster/test_query_log.py@405
PS3, Line 405: r
flake8: F841 local variable 'res' is assigned to but never used
--
To view, visit http://gerrit.cloudera.org:8080/21153
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e3249a8f306cf43de0d6f6586711c779399e83b
Gerrit-Change-Number: 21153
Gerrit-PatchSet: 3
Gerrit-Owner: Jason Fehr <jf...@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-Comment-Date: Thu, 21 Mar 2024 18:44:23 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12913: Refactor Workload Management Custom Cluster Tests
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/21153 )
Change subject: IMPALA-12913: Refactor Workload Management Custom Cluster Tests
......................................................................
IMPALA-12913: Refactor Workload Management Custom Cluster Tests
The custom cluster tests that assert the workload
management functionality to insert completed queries into
the impala_query_log table were inefficient because they
created their own database tables and added data to those
tables.
This patch updates these tests to use the existing tables
in the functional database where possible. The few tests
that need their own tables now have those tables set up in
a database created by the pytest unique_database fixture
instead of using the default database.
A new table has also been added to the functional database.
This table is named zipcode_timezones and contains two
columns, the first having a few zipcodes and the second
having their corresponding timezone. This table can be used
to join the zipcode_incomes and alltimezones tables. This
table is populated by a new csv file in the testdata
directory.
Change-Id: I1e3249a8f306cf43de0d6f6586711c779399e83b
Reviewed-on: http://gerrit.cloudera.org:8080/21153
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
A testdata/data/zipcodes_timezones.csv
M testdata/datasets/functional/functional_schema_template.sql
M tests/custom_cluster/test_query_log.py
A tests/custom_cluster/test_sys_db.py
M tests/util/workload_management.py
5 files changed, 1,086 insertions(+), 490 deletions(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/21153
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I1e3249a8f306cf43de0d6f6586711c779399e83b
Gerrit-Change-Number: 21153
Gerrit-PatchSet: 10
Gerrit-Owner: Jason Fehr <jf...@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>
[Impala-ASF-CR] IMPALA-12913: Refactor Workload Management Custom Cluster Tests
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21153 )
Change subject: IMPALA-12913: Refactor Workload Management Custom Cluster Tests
......................................................................
Patch Set 5:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/15621/ : 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/21153
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e3249a8f306cf43de0d6f6586711c779399e83b
Gerrit-Change-Number: 21153
Gerrit-PatchSet: 5
Gerrit-Owner: Jason Fehr <jf...@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-Comment-Date: Thu, 21 Mar 2024 20:50:00 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12913: Refactor Workload Management Custom Cluser Tests
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21153 )
Change subject: IMPALA-12913: Refactor Workload Management Custom Cluser Tests
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/15542/ : 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/21153
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e3249a8f306cf43de0d6f6586711c779399e83b
Gerrit-Change-Number: 21153
Gerrit-PatchSet: 1
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 15 Mar 2024 23:04:20 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12913: Refactor Workload Management Custom Cluster Tests
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21153 )
Change subject: IMPALA-12913: Refactor Workload Management Custom Cluster Tests
......................................................................
Patch Set 8:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/15683/ : 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/21153
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e3249a8f306cf43de0d6f6586711c779399e83b
Gerrit-Change-Number: 21153
Gerrit-PatchSet: 8
Gerrit-Owner: Jason Fehr <jf...@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-Comment-Date: Tue, 26 Mar 2024 22:53:27 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12913: Refactor Workload Management Custom Cluster Tests
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/21153 )
Change subject: IMPALA-12913: Refactor Workload Management Custom Cluster Tests
......................................................................
Patch Set 6:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/21153/6/tests/custom_cluster/test_query_log.py
File tests/custom_cluster/test_query_log.py:
http://gerrit.cloudera.org:8080/#/c/21153/6/tests/custom_cluster/test_query_log.py@881
PS6, Line 881: TestQueryLogTableAll
This should be TestQueryLogTableBase. Otherwise, it inherits and runs all test methods of TestQueryLogTableAll.
--
To view, visit http://gerrit.cloudera.org:8080/21153
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e3249a8f306cf43de0d6f6586711c779399e83b
Gerrit-Change-Number: 21153
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-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Mon, 25 Mar 2024 18:42:31 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12913: Refactor Workload Management Custom Cluster Tests
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21153 )
Change subject: IMPALA-12913: Refactor Workload Management Custom Cluster Tests
......................................................................
Patch Set 6:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/15636/ : 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/21153
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e3249a8f306cf43de0d6f6586711c779399e83b
Gerrit-Change-Number: 21153
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-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Fri, 22 Mar 2024 19:54:17 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12913: Refactor Workload Management Custom Cluster Tests
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21153 )
Change subject: IMPALA-12913: Refactor Workload Management Custom Cluster Tests
......................................................................
Patch Set 9: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/21153
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e3249a8f306cf43de0d6f6586711c779399e83b
Gerrit-Change-Number: 21153
Gerrit-PatchSet: 9
Gerrit-Owner: Jason Fehr <jf...@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-Comment-Date: Tue, 26 Mar 2024 23:34:32 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12913: Refactor Workload Management Custom Cluser Tests
Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21153 )
Change subject: IMPALA-12913: Refactor Workload Management Custom Cluser Tests
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/21153/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/21153/1//COMMIT_MSG@7
PS1, Line 7: IMPALA-12913: Refactor Workload Management Custom Cluser Tests
typo in the commit message: "Cluster"
--
To view, visit http://gerrit.cloudera.org:8080/21153
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e3249a8f306cf43de0d6f6586711c779399e83b
Gerrit-Change-Number: 21153
Gerrit-PatchSet: 1
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Tue, 19 Mar 2024 15:07:38 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12913: Refactor Workload Management Custom Cluster Tests
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21153 )
Change subject: IMPALA-12913: Refactor Workload Management Custom Cluster Tests
......................................................................
Patch Set 4:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/15620/ : 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/21153
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e3249a8f306cf43de0d6f6586711c779399e83b
Gerrit-Change-Number: 21153
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-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Thu, 21 Mar 2024 20:49:21 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12913: Refactor Workload Management Custom Cluser Tests
Posted by "Jason Fehr (Code Review)" <ge...@cloudera.org>.
Jason Fehr has posted comments on this change. ( http://gerrit.cloudera.org:8080/21153 )
Change subject: IMPALA-12913: Refactor Workload Management Custom Cluser Tests
......................................................................
Patch Set 2:
(6 comments)
http://gerrit.cloudera.org:8080/#/c/21153/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/21153/1//COMMIT_MSG@7
PS1, Line 7: IMPALA-12913: Refactor Workload Management Custom Cluser Tests
> typo in the commit message: "Cluster"
Done
http://gerrit.cloudera.org:8080/#/c/21153/1/testdata/datasets/functional/functional_schema_template.sql
File testdata/datasets/functional/functional_schema_template.sql:
http://gerrit.cloudera.org:8080/#/c/21153/1/testdata/datasets/functional/functional_schema_template.sql@1804
PS1, Line 1804: CREATE EXTERNAL TABLE IF NOT EXISTS {db_name}{db_suffix}.{table_name} (
> Agree, using COLUMNS section might be better.
No reason, just new to this file. Updated to use the existing features to avoid the create table.
http://gerrit.cloudera.org:8080/#/c/21153/1/tests/custom_cluster/test_query_log.py
File tests/custom_cluster/test_query_log.py:
http://gerrit.cloudera.org:8080/#/c/21153/1/tests/custom_cluster/test_query_log.py@44
PS1, Line 44: super(TestQueryLogTableBase, cls).add_test_dimensions()
> Add the following after L44
Done
http://gerrit.cloudera.org:8080/#/c/21153/1/tests/custom_cluster/test_query_log.py@438
PS1, Line 438: client = self.create_impala_client(protocol=vector.get_value('protocol'))
> ImpalaTestSuite and its subclasses initialize 3 clients for each test: clie
I switched to using the default clients to avoid the unnecessary creation of extra clients. A few of the tests do use a second client to ensure that different coordinators can see the rows in the completed queries table, and those clients are created/closed explicitly.
http://gerrit.cloudera.org:8080/#/c/21153/1/tests/custom_cluster/test_query_log.py@614
PS1, Line 614: @pytest.mark.parametrize("buffer_pool_limit", [None, "14.97MB"])
: def test_query_log_table_query_select(self, vector, buffer_pool_limit):
> Contain this test in its own test class, and define buffer_pool_limit test
Done
http://gerrit.cloudera.org:8080/#/c/21153/1/tests/custom_cluster/test_query_log.py@635
PS1, Line 635: client.set_configuration_option("SPOOL_QUERY_RESULTS", "TRUE")
> Can probably remove this, since it is True by default.
https://impala.apache.org/docs/build/html/topics/impala_spool_query_results.html says default is false. Code defaults it to true: https://github.com/apache/impala/blob/1e67480ba6cd4a6f01b369eb11cc2b7330554bde/common/thrift/Query.thrift#L411
Filed https://issues.apache.org/jira/browse/IMPALA-12932 to determine how to handle this discrepency.
I'm going to leave this line in here since I'm not sure what is going to happen with the spool_query_results options (if the default will be changed or if the docs will be changed).
--
To view, visit http://gerrit.cloudera.org:8080/21153
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e3249a8f306cf43de0d6f6586711c779399e83b
Gerrit-Change-Number: 21153
Gerrit-PatchSet: 2
Gerrit-Owner: Jason Fehr <jf...@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-Comment-Date: Thu, 21 Mar 2024 18:43:17 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12913: Refactor Workload Management Custom Cluster Tests
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21153 )
Change subject: IMPALA-12913: Refactor Workload Management Custom Cluster Tests
......................................................................
Patch Set 7:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/15677/ : 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/21153
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e3249a8f306cf43de0d6f6586711c779399e83b
Gerrit-Change-Number: 21153
Gerrit-PatchSet: 7
Gerrit-Owner: Jason Fehr <jf...@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-Comment-Date: Tue, 26 Mar 2024 15:55:33 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12913: Refactor Workload Management Custom Cluster Tests
Posted by "Jason Fehr (Code Review)" <ge...@cloudera.org>.
Hello Riza Suminto, Michael Smith, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/21153
to look at the new patch set (#7).
Change subject: IMPALA-12913: Refactor Workload Management Custom Cluster Tests
......................................................................
IMPALA-12913: Refactor Workload Management Custom Cluster Tests
The custom cluster tests that assert the workload
management functionality to insert completed queries into
the impala_query_log table were inefficient because they
created their own database tables and added data to those
tables.
This patch updates these tests to use the existing tables
in the functional database where possible. The few tests
that need their own tables now have those tables set up in
a database created by the pytest unique_database fixture
instead of using the default database.
A new table has also been added to the functional database.
This table is named zipcode_timezones and contains two
columns, the first having a few zipcodes and the second
having their corresponding timezone. This table can be used
to join the zipcode_incomes and alltimezones tables. This
table is populated by a new csv file in the testdata
directory.
Change-Id: I1e3249a8f306cf43de0d6f6586711c779399e83b
---
A testdata/data/zipcodes_timezones.csv
M testdata/datasets/functional/functional_schema_template.sql
M tests/custom_cluster/test_query_log.py
M tests/util/workload_management.py
4 files changed, 1,053 insertions(+), 491 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/53/21153/7
--
To view, visit http://gerrit.cloudera.org:8080/21153
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1e3249a8f306cf43de0d6f6586711c779399e83b
Gerrit-Change-Number: 21153
Gerrit-PatchSet: 7
Gerrit-Owner: Jason Fehr <jf...@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>
[Impala-ASF-CR] IMPALA-12913: Refactor Workload Management Custom Cluster Tests
Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21153 )
Change subject: IMPALA-12913: Refactor Workload Management Custom Cluster Tests
......................................................................
Patch Set 8: Code-Review+1
(1 comment)
http://gerrit.cloudera.org:8080/#/c/21153/6/tests/custom_cluster/test_query_log.py
File tests/custom_cluster/test_query_log.py:
http://gerrit.cloudera.org:8080/#/c/21153/6/tests/custom_cluster/test_query_log.py@51
PS6, Line 51:
> There are a few fields in the completed queries table that are different ba
Oh right! Might be worth a comment for the next time I forget.
--
To view, visit http://gerrit.cloudera.org:8080/21153
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e3249a8f306cf43de0d6f6586711c779399e83b
Gerrit-Change-Number: 21153
Gerrit-PatchSet: 8
Gerrit-Owner: Jason Fehr <jf...@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-Comment-Date: Tue, 26 Mar 2024 22:39:06 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12913: Refactor Workload Management Custom Cluster Tests
Posted by "Jason Fehr (Code Review)" <ge...@cloudera.org>.
Hello Riza Suminto, Michael Smith, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/21153
to look at the new patch set (#3).
Change subject: IMPALA-12913: Refactor Workload Management Custom Cluster Tests
......................................................................
IMPALA-12913: Refactor Workload Management Custom Cluster Tests
The custom cluster tests that assert the workload
management functionality to insert completed queries into
the impala_query_log table were inefficient because they
created their own database tables and added data to those
tables.
This patch updates these tests to use the existing tables
in the functional database where possible. The few tests
that need their own tables now have those tables set up in
a database created by the pytest unique_database fixture
instead of using the default database.
A new table has also been added to the functional database.
This table is named zipcode_timezones and contains two
columns, the first having a few zipcodes and the second
having their corresponding timezone. This table can be used
to join the zipcode_incomes and alltimezones tables. This
table is populated by a new csv file in the testdata
directory.
Change-Id: I1e3249a8f306cf43de0d6f6586711c779399e83b
---
A testdata/data/zipcodes_timezones.csv
M testdata/datasets/functional/functional_schema_template.sql
M tests/custom_cluster/test_query_log.py
3 files changed, 983 insertions(+), 485 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/53/21153/3
--
To view, visit http://gerrit.cloudera.org:8080/21153
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1e3249a8f306cf43de0d6f6586711c779399e83b
Gerrit-Change-Number: 21153
Gerrit-PatchSet: 3
Gerrit-Owner: Jason Fehr <jf...@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>
[Impala-ASF-CR] IMPALA-12913: Refactor Workload Management Custom Cluster Tests
Posted by "Jason Fehr (Code Review)" <ge...@cloudera.org>.
Hello Riza Suminto, Michael Smith, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/21153
to look at the new patch set (#5).
Change subject: IMPALA-12913: Refactor Workload Management Custom Cluster Tests
......................................................................
IMPALA-12913: Refactor Workload Management Custom Cluster Tests
The custom cluster tests that assert the workload
management functionality to insert completed queries into
the impala_query_log table were inefficient because they
created their own database tables and added data to those
tables.
This patch updates these tests to use the existing tables
in the functional database where possible. The few tests
that need their own tables now have those tables set up in
a database created by the pytest unique_database fixture
instead of using the default database.
A new table has also been added to the functional database.
This table is named zipcode_timezones and contains two
columns, the first having a few zipcodes and the second
having their corresponding timezone. This table can be used
to join the zipcode_incomes and alltimezones tables. This
table is populated by a new csv file in the testdata
directory.
Change-Id: I1e3249a8f306cf43de0d6f6586711c779399e83b
---
A testdata/data/zipcodes_timezones.csv
M testdata/datasets/functional/functional_schema_template.sql
M tests/custom_cluster/test_query_log.py
3 files changed, 982 insertions(+), 485 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/53/21153/5
--
To view, visit http://gerrit.cloudera.org:8080/21153
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1e3249a8f306cf43de0d6f6586711c779399e83b
Gerrit-Change-Number: 21153
Gerrit-PatchSet: 5
Gerrit-Owner: Jason Fehr <jf...@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>
[Impala-ASF-CR] IMPALA-12913: Refactor Workload Management Custom Cluster Tests
Posted by "Jason Fehr (Code Review)" <ge...@cloudera.org>.
Hello Riza Suminto, Michael Smith, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/21153
to look at the new patch set (#6).
Change subject: IMPALA-12913: Refactor Workload Management Custom Cluster Tests
......................................................................
IMPALA-12913: Refactor Workload Management Custom Cluster Tests
The custom cluster tests that assert the workload
management functionality to insert completed queries into
the impala_query_log table were inefficient because they
created their own database tables and added data to those
tables.
This patch updates these tests to use the existing tables
in the functional database where possible. The few tests
that need their own tables now have those tables set up in
a database created by the pytest unique_database fixture
instead of using the default database.
A new table has also been added to the functional database.
This table is named zipcode_timezones and contains two
columns, the first having a few zipcodes and the second
having their corresponding timezone. This table can be used
to join the zipcode_incomes and alltimezones tables. This
table is populated by a new csv file in the testdata
directory.
Change-Id: I1e3249a8f306cf43de0d6f6586711c779399e83b
---
A testdata/data/zipcodes_timezones.csv
M testdata/datasets/functional/functional_schema_template.sql
M tests/custom_cluster/test_query_log.py
M tests/util/workload_management.py
4 files changed, 1,053 insertions(+), 491 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/53/21153/6
--
To view, visit http://gerrit.cloudera.org:8080/21153
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1e3249a8f306cf43de0d6f6586711c779399e83b
Gerrit-Change-Number: 21153
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-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
[Impala-ASF-CR] IMPALA-12913: Refactor Workload Management Custom Cluster Tests
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21153 )
Change subject: IMPALA-12913: Refactor Workload Management Custom Cluster Tests
......................................................................
Patch Set 9: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/21153
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e3249a8f306cf43de0d6f6586711c779399e83b
Gerrit-Change-Number: 21153
Gerrit-PatchSet: 9
Gerrit-Owner: Jason Fehr <jf...@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-Comment-Date: Wed, 27 Mar 2024 04:46:36 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12913: Refactor Workload Management Custom Cluster Tests
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/21153 )
Change subject: IMPALA-12913: Refactor Workload Management Custom Cluster Tests
......................................................................
Patch Set 8: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/21153
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e3249a8f306cf43de0d6f6586711c779399e83b
Gerrit-Change-Number: 21153
Gerrit-PatchSet: 8
Gerrit-Owner: Jason Fehr <jf...@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-Comment-Date: Tue, 26 Mar 2024 23:25:49 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12913: Refactor Workload Management Custom Cluster Tests
Posted by "Jason Fehr (Code Review)" <ge...@cloudera.org>.
Jason Fehr has posted comments on this change. ( http://gerrit.cloudera.org:8080/21153 )
Change subject: IMPALA-12913: Refactor Workload Management Custom Cluster Tests
......................................................................
Patch Set 8:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/21153/6/tests/custom_cluster/test_query_log.py
File tests/custom_cluster/test_query_log.py:
http://gerrit.cloudera.org:8080/#/c/21153/6/tests/custom_cluster/test_query_log.py@51
PS6, Line 51:
> I wouldn't expect any difference in behavior between Beeswax and HS2, from
There are a few fields in the completed queries table that are different based on the client protocol.
The TestQueryLogTableAll class is the only test class that runs using both the beeswax and hs2 dimensions. This class contains unique tests that I would like to test on both client protocols to cover any corner cases. For example, there is only 1 dml query and 1 ddl that are asserted. I test on both protocols for those. I also test one invalid query and all combinations of ignore queries to make sure that changing the client protocol doesn't actually run on some unexpected code path that should not be executed. I was very intentional to only do duplication where I thought it added unique test coverage.
http://gerrit.cloudera.org:8080/#/c/21153/6/tests/custom_cluster/test_query_log.py@97
PS6, Line 97: # causing the execution to take a very long time.
> Could you comment that we use async to avoid fetching the large result stri
Done
http://gerrit.cloudera.org:8080/#/c/21153/6/tests/custom_cluster/test_query_log.py@383
PS6, Line 383: cluster_size=3,
> This isn't specific to query_log, could we move the sys db tests to a new f
Done
http://gerrit.cloudera.org:8080/#/c/21153/7/tests/custom_cluster/test_query_log.py
File tests/custom_cluster/test_query_log.py:
http://gerrit.cloudera.org:8080/#/c/21153/7/tests/custom_cluster/test_query_log.py@208
PS7, Line 208: res = client.execute(select_sql)
> Why do you sleep between runs?
The original issue is that running queries too quickly did not cause the cache to be populated. I modified this code to check the complete-queries.written metric after each query. That will ensure there is some down time between queries.
http://gerrit.cloudera.org:8080/#/c/21153/7/tests/custom_cluster/test_query_log.py@214
PS7, Line 214: self.cluster.get_first_impalad().service.wait_for_metric_value(
> What are you actually waiting on. Is there a data-cache metric we can watch
Time is needed for the cache to be populated or else it is not used. I modified this code to check a metric instead of sleeping.
--
To view, visit http://gerrit.cloudera.org:8080/21153
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e3249a8f306cf43de0d6f6586711c779399e83b
Gerrit-Change-Number: 21153
Gerrit-PatchSet: 8
Gerrit-Owner: Jason Fehr <jf...@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-Comment-Date: Tue, 26 Mar 2024 22:28:52 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12913: Refactor Workload Management Custom Cluster Tests
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/21153 )
Change subject: IMPALA-12913: Refactor Workload Management Custom Cluster Tests
......................................................................
Patch Set 3:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/21153/1/tests/custom_cluster/test_query_log.py
File tests/custom_cluster/test_query_log.py:
http://gerrit.cloudera.org:8080/#/c/21153/1/tests/custom_cluster/test_query_log.py@635
PS1, Line 635: catalogd_args="--enable_workload_m
> https://impala.apache.org/docs/build/html/topics/impala_spool_query_results
Ack
http://gerrit.cloudera.org:8080/#/c/21153/3/tests/custom_cluster/test_query_log.py
File tests/custom_cluster/test_query_log.py:
http://gerrit.cloudera.org:8080/#/c/21153/3/tests/custom_cluster/test_query_log.py@822
PS3, Line 822: TestQueryLogTableAll
I think this should extend TestQueryLogTableBase?
--
To view, visit http://gerrit.cloudera.org:8080/21153
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e3249a8f306cf43de0d6f6586711c779399e83b
Gerrit-Change-Number: 21153
Gerrit-PatchSet: 3
Gerrit-Owner: Jason Fehr <jf...@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-Comment-Date: Thu, 21 Mar 2024 20:28:43 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12913: Refactor Workload Management Custom Cluster Tests
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21153 )
Change subject: IMPALA-12913: Refactor Workload Management Custom Cluster Tests
......................................................................
Patch Set 3:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/15613/ : 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/21153
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e3249a8f306cf43de0d6f6586711c779399e83b
Gerrit-Change-Number: 21153
Gerrit-PatchSet: 3
Gerrit-Owner: Jason Fehr <jf...@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-Comment-Date: Thu, 21 Mar 2024 19:06:44 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12913: Refactor Workload Management Custom Cluser Tests
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/21153 )
Change subject: IMPALA-12913: Refactor Workload Management Custom Cluser Tests
......................................................................
Patch Set 1:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/21153/1/testdata/datasets/functional/functional_schema_template.sql
File testdata/datasets/functional/functional_schema_template.sql:
http://gerrit.cloudera.org:8080/#/c/21153/1/testdata/datasets/functional/functional_schema_template.sql@1804
PS1, Line 1804: CREATE EXTERNAL TABLE IF NOT EXISTS {db_name}{db_suffix}.{table_name} (
> There seem to be support for all this above with COLUMNS, ROW_FORMAT, DEPEN
Agree, using COLUMNS section might be better.
http://gerrit.cloudera.org:8080/#/c/21153/1/tests/custom_cluster/test_query_log.py
File tests/custom_cluster/test_query_log.py:
http://gerrit.cloudera.org:8080/#/c/21153/1/tests/custom_cluster/test_query_log.py@44
PS1, Line 44: super(TestQueryLogTableBase, cls).add_test_dimensions()
Add the following after L44
cls.ImpalaTestMatrix.add_dimension(create_single_exec_option_dimension())
This will only test with disable_codegen_options=False.
http://gerrit.cloudera.org:8080/#/c/21153/1/tests/custom_cluster/test_query_log.py@438
PS1, Line 438: client = self.create_impala_client(protocol=vector.get_value('protocol'))
> Why do you create a client?
ImpalaTestSuite and its subclasses initialize 3 clients for each test: client (beeswax), hs2_client (HS2), and hs2_http_client (HS2 over HTTP)
https://github.com/apache/impala/blob/4477398ae46415d3fb32db2a8fd5e6d2060cbd3f/tests/common/impala_test_suite.py#L322-L353
run_test_case() use them, and they are closed automatically after test run complete. So no need to declare try-finally. But the benefit stops there.
Reusing them is probably good if test only need one client of that kind. But once the test gets complex (using 2 client like this method, or using create_client_for_nth_impalad), I think it is fine to doo it this way too.
http://gerrit.cloudera.org:8080/#/c/21153/1/tests/custom_cluster/test_query_log.py@614
PS1, Line 614: @pytest.mark.parametrize("buffer_pool_limit", [None, "14.97MB"])
: def test_query_log_table_query_select(self, vector, buffer_pool_limit):
Contain this test in its own test class, and define buffer_pool_limit test dimension just for this. For example:
class TestQueryLogTableBufferPool(CustomClusterTestSuite):
"""Tests to assert the query log table is correctly populated."""
@classmethod
def add_test_dimensions(cls):
super(TestQueryLogTableBufferPool, cls).add_test_dimensions()
add_exec_option_dimension(cls, 'buffer_pool_limit', [None, '16.05MB'])
add_mandatory_exec_option(cls, 'max_mem_estimate_for_admission', '10MB')
@CustomClusterTestSuite.with_args(impalad_args="--enable_workload_mgmt "
"--query_log_write_interval_s=1 "
"--cluster_id=test_query_hist_1 "
"--shutdown_grace_period_s=10 "
"--shutdown_deadline_s=60 "
"--scratch_dirs={0}:5G"
.format(SCRATCH_DIR),
catalogd_args="--enable_workload_mgmt",
impalad_graceful_shutdown=True)
def test_query_log_table_query_select(self, vector, unique_database):
"""Asserts the values written to the query log table match the values from the
query profile. If the buffer_pool_limit parameter is not None, then this test
requires that the query spills to disk to assert that the spill metrics are correct
in the completed queries table."""
tbl_name = unique_database + ".test_query_log"
client_protocol = vector.get_value('protocol')
buffer_pool_limit = vector.get_value('exec_option')['buffer_pool_limit']
client = self.create_impala_client(protocol=client_protocol)
http://gerrit.cloudera.org:8080/#/c/21153/1/tests/custom_cluster/test_query_log.py@635
PS1, Line 635: client.set_configuration_option("SPOOL_QUERY_RESULTS", "TRUE")
Can probably remove this, since it is True by default.
--
To view, visit http://gerrit.cloudera.org:8080/21153
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e3249a8f306cf43de0d6f6586711c779399e83b
Gerrit-Change-Number: 21153
Gerrit-PatchSet: 1
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Mon, 18 Mar 2024 22:28:06 +0000
Gerrit-HasComments: Yes