Riza Suminto has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/22511 )

Change subject: IMPALA-13772: Fix Workload Management DMLs Timeouts
......................................................................


Patch Set 6:

(5 comments)

I take another look at the test and have some additional feedback.

http://gerrit.cloudera.org:8080/#/c/22511/2/be/src/service/workload-management-worker.cc
File be/src/service/workload-management-worker.cc:

http://gerrit.cloudera.org:8080/#/c/22511/2/be/src/service/workload-management-worker.cc@776
PS2, Line 776:     opts[TImpalaQueryOptions::MAX_STATEMENT_LENGTH_BYTES] =
> Other timeouts will kick in to prevent the query from waiting too long.  Sp
Ack


http://gerrit.cloudera.org:8080/#/c/22511/6/tests/custom_cluster/test_query_log.py
File tests/custom_cluster/test_query_log.py:

http://gerrit.cloudera.org:8080/#/c/22511/6/tests/custom_cluster/test_query_log.py@1137
PS6, Line 1137: HS2TestSuite
I think you can avoid HS2TestSuite and use more generic ImpylaHS2Connection 
(the self.hs2_client) + execute_async() + fetch() instead. We kind of converge 
to use Impyla for simple querying.

I know ImpylaHS2Connection has not support some feature yet 
(https://gerrit.cloudera.org/c/22362), but in this test, you only run 1 sync 
query and 1 async query. So it is doable.


http://gerrit.cloudera.org:8080/#/c/22511/6/tests/custom_cluster/test_query_log.py@1141
PS6, Line 1141:   @classmethod
              :   def add_test_dimensions(cls):
              :     super(TestQueryLogTableHS2, cls).add_test_dimensions()
              :     cls.ImpalaTestMatrix.add_constraint(lambda v:
              :         v.get_value('protocol') == 'hs2')
test_query_queued does not use vector fixture, so this override is not needed.


http://gerrit.cloudera.org:8080/#/c/22511/6/tests/custom_cluster/test_query_log.py@1152
PS6, Line 1152: disable_log_buffering=True
This test does not inspect any impalad logs, so disabling log buffering is not 
needed.


http://gerrit.cloudera.org:8080/#/c/22511/6/tests/custom_cluster/test_query_log.py@1164
PS6, Line 1164:          4. Wait 12 seconds to ensure the default 10 second 
FETCH_ROWS_TIMEOUT_MS does not
              :               cause the insert query to fail.
You can make this test run shorter while keeping the assertion if you set low 
default fetch_rows_timeout_ms

  default_query_options=[('fetch_rows_timeout_ms', '1000')]



--
To view, visit http://gerrit.cloudera.org:8080/22511
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0cc7fbce40eadfb253d8cff5cbb83e2ad63a979f
Gerrit-Change-Number: 22511
Gerrit-PatchSet: 6
Gerrit-Owner: Jason Fehr <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Jason Fehr <[email protected]>
Gerrit-Reviewer: Michael Smith <[email protected]>
Gerrit-Reviewer: Riza Suminto <[email protected]>
Gerrit-Comment-Date: Fri, 21 Feb 2025 23:08:33 +0000
Gerrit-HasComments: Yes

Reply via email to